From 345a8534ff39cc9550cbacabe2b3fffe20293508 Mon Sep 17 00:00:00 2001 From: Thomas Lenz Date: Thu, 22 Jun 2017 14:26:15 +0200 Subject: implement a workaround to fix XXE and SSRF problems in an old XMLStreamParser implementation of a third party library --- .../filter/MoccaHttpServletRequestWrapper.java | 72 ++++++++++++ .../egiz/bku/online/filter/StalSecurityFilter.java | 122 +++++++++++++++++++++ .../at/gv/egiz/mocca/id/DataURLServerServlet.java | 13 ++- BKUOnline/src/main/webapp/WEB-INF/web.xml | 8 ++ 4 files changed, 214 insertions(+), 1 deletion(-) create mode 100644 BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/MoccaHttpServletRequestWrapper.java create mode 100644 BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/StalSecurityFilter.java (limited to 'BKUOnline') diff --git a/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/MoccaHttpServletRequestWrapper.java b/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/MoccaHttpServletRequestWrapper.java new file mode 100644 index 00000000..8901969d --- /dev/null +++ b/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/MoccaHttpServletRequestWrapper.java @@ -0,0 +1,72 @@ +package at.gv.egiz.bku.online.filter; + +import java.io.BufferedReader; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStreamReader; + +import javax.servlet.ReadListener; +import javax.servlet.ServletInputStream; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; + +import org.apache.commons.io.IOUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + + +import at.gv.egiz.bku.binding.HttpUtil; + + +public class MoccaHttpServletRequestWrapper extends HttpServletRequestWrapper { + + private static Logger log = LoggerFactory.getLogger(MoccaHttpServletRequestWrapper.class); + + private final byte[] body; + private final String charset; + + public MoccaHttpServletRequestWrapper(HttpServletRequest request) throws IOException { + super(request); + + String ct = request.getHeader(HttpUtil.HTTP_HEADER_CONTENT_TYPE.toLowerCase()); + charset = HttpUtil.getCharset(ct, true); + + byte[] result = null; + try { + result = IOUtils.toByteArray(request.getReader(), charset); + + } catch (IOException e) { + log.error("Can not copy input stream!!!!!", e); + throw new IOException("Can not copy input stream!!!!!", e); + + } finally { + body = result; + + } + } + + public boolean isInputStreamAvailable() { + return (body != null && body.length > 0); + + } + + @Override + public BufferedReader getReader() throws IOException { + return new BufferedReader(new InputStreamReader(getInputStream(), charset)); + + } + + @Override + public ServletInputStream getInputStream() throws IOException { + final ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(body); + return new ServletInputStream() { + + @Override + public int read() throws IOException { + return byteArrayInputStream.read(); + } + + }; + + } +} diff --git a/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/StalSecurityFilter.java b/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/StalSecurityFilter.java new file mode 100644 index 00000000..0e98cb79 --- /dev/null +++ b/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/StalSecurityFilter.java @@ -0,0 +1,122 @@ +package at.gv.egiz.bku.online.filter; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; + +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletInputStream; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.stream.XMLStreamException; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.xml.sax.SAXException; + + +public class StalSecurityFilter implements Filter { + + private static Logger log = LoggerFactory.getLogger(StalSecurityFilter.class); + + @Override + public void init(FilterConfig filterConfig) throws ServletException { + log.info("Initialize STAL Service security filter"); + + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) + throws IOException, ServletException { + + if (request instanceof HttpServletRequest) { + try { + MoccaHttpServletRequestWrapper stalHttpReq = new MoccaHttpServletRequestWrapper((HttpServletRequest) request); + + if (stalHttpReq.isInputStreamAvailable()) { + log.trace("Validate STAL request ... "); + validateStalRequest(stalHttpReq.getInputStream()); + log.trace("Validate of STAL request completed"); + + } + + chain.doFilter(stalHttpReq, response); + + } catch (XMLStreamException e) { + log.error("XML data validation FAILED with msg: " + e.getMessage(), e); + sendErrorToResponse(e, response); + + } catch (IOException e) { + log.error("Can not process InputStream from STAL request"); + sendErrorToResponse(e, response); + + } + + } else { + log.error("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + log.warn("STAL request is processed WITHOUT security checks!!!!"); + log.error("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + chain.doFilter(request, response); + + } + } + + @Override + public void destroy() { + // TODO Auto-generated method stub + + } + + private void sendErrorToResponse(Exception e, ServletResponse response) throws IOException { + if (response instanceof HttpServletResponse) { + ((HttpServletResponse)response).sendError(HttpServletResponse.SC_BAD_REQUEST, e.getMessage()); + + } else + log.error("Can not response with http error message"); + + } + + private void validateStalRequest(InputStream is) throws XMLStreamException, IOException { + + DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + + try { + dbf.setFeature("http://xml.org/sax/features/external-general-entities", false); + dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + + } catch (ParserConfigurationException e) { + log.error("Can NOT set Xerces parser security features. -> XML parsing is possible insecure!!!! ", e); + + } + + try { + //validate input stream + dbf.newDocumentBuilder().parse(is); + + } catch (SAXException e) { + log.error("XML data validation FAILED with msg: " + e.getMessage(), e); + throw new XMLStreamException("XML data validation FAILED with msg: " + e.getMessage(), e); + + } catch (ParserConfigurationException e) { + log.error("XML data validation FAILED with msg: " + e.getMessage(), e); + throw new XMLStreamException("XML data validation FAILED with msg: " + e.getMessage(), e); + + } catch (IOException e) { + log.error("XML data validation FAILED with msg: " + e.getMessage(), e); + throw new XMLStreamException("XML data validation FAILED with msg: " + e.getMessage(), e); + + } + + } + +} diff --git a/BKUOnline/src/main/java/at/gv/egiz/mocca/id/DataURLServerServlet.java b/BKUOnline/src/main/java/at/gv/egiz/mocca/id/DataURLServerServlet.java index 7dd2cd22..37889ae5 100644 --- a/BKUOnline/src/main/java/at/gv/egiz/mocca/id/DataURLServerServlet.java +++ b/BKUOnline/src/main/java/at/gv/egiz/mocca/id/DataURLServerServlet.java @@ -135,7 +135,6 @@ public class DataURLServerServlet extends HttpServlet { } SLUnmarshaller slUnmarshaller = new SLUnmarshaller(); - DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); dbf.setNamespaceAware(true); dbf.setSchema(slUnmarshaller.getSlSchema()); @@ -153,6 +152,18 @@ public class DataURLServerServlet extends HttpServlet { "(see http://www.w3.org/TR/xmldsig-bestpractices/#be-aware-schema-normalization)", e); } + try { + dbf.setFeature("http://xml.org/sax/features/external-general-entities", false); + dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + + } catch (ParserConfigurationException e) { + log.error("Can NOT set SAX parser security features. -> XML parsing is possible insecure!!!! ", e); + + } + + DocumentBuilder documentBuilder; try { documentBuilder = dbf.newDocumentBuilder(); diff --git a/BKUOnline/src/main/webapp/WEB-INF/web.xml b/BKUOnline/src/main/webapp/WEB-INF/web.xml index 5033cc5e..5779fc97 100644 --- a/BKUOnline/src/main/webapp/WEB-INF/web.xml +++ b/BKUOnline/src/main/webapp/WEB-INF/web.xml @@ -175,6 +175,14 @@ RequestIdFilter at.gv.egiz.bku.online.webapp.TransactionIdFilter + + StalSecurityFilter + at.gv.egiz.bku.online.filter.StalSecurityFilter + + + StalSecurityFilter + STALService + RequestIdFilter HTTPSecurityLayerServlet -- cgit v1.2.3 From 0603c0fbdfe028113431c65590b6e7e28929f6f6 Mon Sep 17 00:00:00 2001 From: Thomas Lenz Date: Fri, 23 Jun 2017 06:50:14 +0200 Subject: some small refactoring and code documentation --- .../filter/MoccaHttpServletRequestWrapper.java | 1 - .../egiz/bku/online/filter/StalSecurityFilter.java | 48 ++-------------------- .../at/gv/egiz/mocca/id/DataURLServerServlet.java | 16 ++------ 3 files changed, 7 insertions(+), 58 deletions(-) (limited to 'BKUOnline') diff --git a/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/MoccaHttpServletRequestWrapper.java b/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/MoccaHttpServletRequestWrapper.java index 8901969d..d01f8128 100644 --- a/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/MoccaHttpServletRequestWrapper.java +++ b/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/MoccaHttpServletRequestWrapper.java @@ -5,7 +5,6 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStreamReader; -import javax.servlet.ReadListener; import javax.servlet.ServletInputStream; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; diff --git a/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/StalSecurityFilter.java b/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/StalSecurityFilter.java index 0e98cb79..356401b6 100644 --- a/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/StalSecurityFilter.java +++ b/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/StalSecurityFilter.java @@ -1,26 +1,20 @@ package at.gv.egiz.bku.online.filter; -import java.io.ByteArrayInputStream; import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; - import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; import javax.servlet.ServletException; -import javax.servlet.ServletInputStream; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.ParserConfigurationException; import javax.xml.stream.XMLStreamException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.xml.sax.SAXException; + +import at.gv.egiz.dom.DOMUtils; public class StalSecurityFilter implements Filter { @@ -43,7 +37,7 @@ public class StalSecurityFilter implements Filter { if (stalHttpReq.isInputStreamAvailable()) { log.trace("Validate STAL request ... "); - validateStalRequest(stalHttpReq.getInputStream()); + DOMUtils.validateXMLAgainstXXEAndSSRFAttacks(stalHttpReq.getInputStream()); log.trace("Validate of STAL request completed"); } @@ -71,7 +65,6 @@ public class StalSecurityFilter implements Filter { @Override public void destroy() { - // TODO Auto-generated method stub } @@ -83,40 +76,5 @@ public class StalSecurityFilter implements Filter { log.error("Can not response with http error message"); } - - private void validateStalRequest(InputStream is) throws XMLStreamException, IOException { - - DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); - - try { - dbf.setFeature("http://xml.org/sax/features/external-general-entities", false); - dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - - } catch (ParserConfigurationException e) { - log.error("Can NOT set Xerces parser security features. -> XML parsing is possible insecure!!!! ", e); - - } - - try { - //validate input stream - dbf.newDocumentBuilder().parse(is); - - } catch (SAXException e) { - log.error("XML data validation FAILED with msg: " + e.getMessage(), e); - throw new XMLStreamException("XML data validation FAILED with msg: " + e.getMessage(), e); - - } catch (ParserConfigurationException e) { - log.error("XML data validation FAILED with msg: " + e.getMessage(), e); - throw new XMLStreamException("XML data validation FAILED with msg: " + e.getMessage(), e); - - } catch (IOException e) { - log.error("XML data validation FAILED with msg: " + e.getMessage(), e); - throw new XMLStreamException("XML data validation FAILED with msg: " + e.getMessage(), e); - - } - } - } diff --git a/BKUOnline/src/main/java/at/gv/egiz/mocca/id/DataURLServerServlet.java b/BKUOnline/src/main/java/at/gv/egiz/mocca/id/DataURLServerServlet.java index 37889ae5..d34ead45 100644 --- a/BKUOnline/src/main/java/at/gv/egiz/mocca/id/DataURLServerServlet.java +++ b/BKUOnline/src/main/java/at/gv/egiz/mocca/id/DataURLServerServlet.java @@ -65,6 +65,7 @@ import at.gv.egiz.bku.slcommands.impl.SLCommandImpl; import at.gv.egiz.bku.slexceptions.SLCommandException; import at.gv.egiz.bku.utils.DebugInputStream; import at.gv.egiz.bku.utils.StreamUtil; +import at.gv.egiz.dom.DOMUtils; import at.gv.egiz.org.apache.tomcat.util.http.AcceptLanguage; import at.gv.egiz.slbinding.SLUnmarshaller; @@ -152,18 +153,9 @@ public class DataURLServerServlet extends HttpServlet { "(see http://www.w3.org/TR/xmldsig-bestpractices/#be-aware-schema-normalization)", e); } - try { - dbf.setFeature("http://xml.org/sax/features/external-general-entities", false); - dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - - } catch (ParserConfigurationException e) { - log.error("Can NOT set SAX parser security features. -> XML parsing is possible insecure!!!! ", e); - - } - - + //set XML parser flags to prevent XXE, XEE and SSRF attacks + DOMUtils.setXMLParserFlagsAgainstXXEAndSSRFAttacks(dbf); + DocumentBuilder documentBuilder; try { documentBuilder = dbf.newDocumentBuilder(); -- cgit v1.2.3