diff --git a/xml/src/main/java/org/seamless/xml/DOMParser.java b/xml/src/main/java/org/seamless/xml/DOMParser.java index c4e2711..1e8acf3 100644 --- a/xml/src/main/java/org/seamless/xml/DOMParser.java +++ b/xml/src/main/java/org/seamless/xml/DOMParser.java @@ -136,6 +136,15 @@ public DocumentBuilderFactory createFactory(boolean validating) throws ParserExc factory.setFeature("http://apache.org/xml/features/xinclude/fixup-base-uris", false); factory.setFeature("http://apache.org/xml/features/xinclude/fixup-language", false); + // Configure parser to prevent XXE attacks + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + factory.setXIncludeAware(false); + factory.setExpandEntityReferences(false); + + // Good idea to set a schema when you want to validate! Tell me, how does it work // without a schema?! factory.setSchema(getSchema()); diff --git a/xml/src/main/java/org/seamless/xml/SAXParser.java b/xml/src/main/java/org/seamless/xml/SAXParser.java index 88ade83..5679f0e 100644 --- a/xml/src/main/java/org/seamless/xml/SAXParser.java +++ b/xml/src/main/java/org/seamless/xml/SAXParser.java @@ -65,16 +65,23 @@ public void setContentHandler(ContentHandler handler) { protected XMLReader create() { try { + SAXParserFactory factory = SAXParserFactory.newInstance(); + + // Configure factory to prevent XXE attacks + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setXIncludeAware(false); + + factory.setNamespaceAware(true); + if (getSchemaSources() != null) { - // Jump through all the hoops and create a validating reader - SAXParserFactory factory = SAXParserFactory.newInstance(); - factory.setNamespaceAware(true); factory.setSchema(createSchema(getSchemaSources())); - XMLReader xmlReader = factory.newSAXParser().getXMLReader(); - xmlReader.setErrorHandler(getErrorHandler()); - return xmlReader; } - return XMLReaderFactory.createXMLReader(); + + XMLReader xmlReader = factory.newSAXParser().getXMLReader(); + xmlReader.setErrorHandler(getErrorHandler()); + return xmlReader; } catch (Exception ex) { throw new RuntimeException(ex); } diff --git a/xml/src/test/java/org/seamless/test/xml/SAXParserTest.java b/xml/src/test/java/org/seamless/test/xml/SAXParserTest.java new file mode 100644 index 0000000..cccccfe --- /dev/null +++ b/xml/src/test/java/org/seamless/test/xml/SAXParserTest.java @@ -0,0 +1,50 @@ +/* +* Copyright (C) 2018 Matthew Piggott +* +* The contents of this file are subject to the terms of either the GNU +* Lesser General Public License Version 2 or later ("LGPL") or the +* Common Development and Distribution License Version 1 or later +* ("CDDL") (collectively, the "License"). You may not use this file +* except in compliance with the License. See LICENSE.txt for more +* information. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. +*/ +package org.seamless.test.xml; + +import static org.testng.Assert.assertTrue; +import static org.testng.Assert.fail; + +import java.io.IOException; +import java.io.InputStream; + +import org.seamless.xml.ParserException; +import org.seamless.xml.SAXParser; +import org.testng.annotations.Test; +import org.xml.sax.InputSource; + +/** + * @author Matthew Piggott + */ +public class SAXParserTest { + + @Test + public void testXxe() throws IOException { + SAXParser parser = new SAXParser(); + + InputStream in = null; + try { + in = getClass().getResourceAsStream("/org/seamless/test/xml/xxe.xml"); + parser.parse(new InputSource(in)); + fail("Expected exception thrown"); + } catch (ParserException e) { + assertTrue(e.getMessage().contains("DOCTYPE is disallowed")); + } finally { + if (in != null) { + in.close(); + } + } + } +} diff --git a/xml/src/test/resources/org/seamless/test/xml/xxe.xml b/xml/src/test/resources/org/seamless/test/xml/xxe.xml new file mode 100644 index 0000000..628889d --- /dev/null +++ b/xml/src/test/resources/org/seamless/test/xml/xxe.xml @@ -0,0 +1,7 @@ + + + + +]> +&xxe;&xxe-url;