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/src') 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