Add a new method to obtain a secure XMLEventReader.

uhafner · Sep 16, 2022 · 3be23b0 · 3be23b0
1 parent 0a805e5
commit 3be23b0
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 11 deletions.
diff --git a/src/main/java/edu/hm/hafner/util/SecureXmlParserFactory.java b/src/main/java/edu/hm/hafner/util/SecureXmlParserFactory.java
@@ -8,6 +8,7 @@
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.parsers.SAXParser;
 import javax.xml.parsers.SAXParserFactory;
+import javax.xml.stream.XMLEventReader;
 import javax.xml.stream.XMLInputFactory;
 import javax.xml.stream.XMLStreamException;
 import javax.xml.stream.XMLStreamReader;
@@ -33,8 +34,10 @@
  * containing a reference to an external entity is processed by a weakly configured XML parser.
  *
  * @author Ullrich Hafner
- * @see <a href="https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html">XML External Entity Prevention Cheat Sheet</a>
- * @see <a href="https://rules.sonarsource.com/java/RSPEC-2755">XML parsers should not be vulnerable to XXE attacks</a>
+ * @see <a href="https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html">XML
+ *         External Entity Prevention Cheat Sheet</a>
+ * @see <a href="https://rules.sonarsource.com/java/RSPEC-2755">XML parsers should not be vulnerable to XXE
+ *         attacks</a>
  */
 public class SecureXmlParserFactory {
     /**
@@ -117,7 +120,7 @@ private void setFeature(final DocumentBuilderFactory factory, final String enabl
     }
 
     private void clearAttributes(final DocumentBuilderFactory factory) {
-        for (String securityAttribute: DISABLED_ATTRIBUTES) {
+        for (String securityAttribute : DISABLED_ATTRIBUTES) {
             try {
                 factory.setAttribute(securityAttribute, CLEAR_ATTRIBUTE);
             }
@@ -128,7 +131,7 @@ private void clearAttributes(final DocumentBuilderFactory factory) {
     }
 
     private void clearAttributes(final TransformerFactory transformerFactory) {
-        for (String securityAttribute: DISABLED_ATTRIBUTES) {
+        for (String securityAttribute : DISABLED_ATTRIBUTES) {
             try {
                 transformerFactory.setAttribute(securityAttribute, CLEAR_ATTRIBUTE);
             }
@@ -165,10 +168,11 @@ SAXParserFactory createSaxParserFactory() {
     /**
      * Secure the {@link SAXParser} so that it does not resolve external entities.
      *
-     * @param parser the parser to secure
+     * @param parser
+     *         the parser to secure
      */
     private void secureParser(final SAXParser parser) {
-        for (String securityAttribute: DISABLED_ATTRIBUTES) {
+        for (String securityAttribute : DISABLED_ATTRIBUTES) {
             try {
                 parser.setProperty(securityAttribute, CLEAR_ATTRIBUTE);
             }
@@ -216,16 +220,37 @@ public void configureSaxParserFactory(final SAXParserFactory factory) {
      */
     public XMLStreamReader createXmlStreamReader(final Reader reader) {
         try {
-            var factory = createXmlInputFactory();
-            factory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
-            factory.setProperty(SUPPORTING_EXTERNAL_ENTITIES, false);
-            return factory.createXMLStreamReader(reader);
+            return createSecureInputFactory().createXMLStreamReader(reader);
         }
         catch (XMLStreamException exception) {
             throw new IllegalArgumentException("Can't create instance of XMLStreamReader", exception);
         }
     }
 
+    /**
+     * Creates a new instance of a {@link XMLStreamReader} that does not resolve external entities.
+     *
+     * @param reader
+     *         the reader to wrap
+     *
+     * @return a new instance of a {@link XMLStreamReader}
+     */
+    public XMLEventReader createXmlEventReader(final Reader reader) {
+        try {
+            return createSecureInputFactory().createXMLEventReader(reader);
+        }
+        catch (XMLStreamException exception) {
+            throw new IllegalArgumentException("Can't create instance of XMLEventReader", exception);
+        }
+    }
+
+    private XMLInputFactory createSecureInputFactory() {
+        var factory = createXmlInputFactory();
+        factory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
+        factory.setProperty(SUPPORTING_EXTERNAL_ENTITIES, false);
+        return factory;
+    }
+
     @VisibleForTesting
     XMLInputFactory createXmlInputFactory() {
         return XMLInputFactory.newInstance();

diff --git a/src/test/java/edu/hm/hafner/util/SecureXmlParserFactoryTest.java b/src/test/java/edu/hm/hafner/util/SecureXmlParserFactoryTest.java
@@ -97,7 +97,7 @@ void shouldFailWithParsingExceptionIfParsingBrokenDocument() throws SAXException
     }
 
     @Test
-    void shouldCreateXmlInputFactory() throws XMLStreamException {
+    void shouldCreateXmlStreamReader() throws XMLStreamException {
         var factory = spy(new SecureXmlParserFactory());
 
         assertThat(factory.createXmlStreamReader(createEmptyXmlReader())).isNotNull();
@@ -109,9 +109,23 @@ void shouldCreateXmlInputFactory() throws XMLStreamException {
                 .withMessage("Can't create instance of XMLStreamReader");
     }
 
+    @Test
+    void shouldCreateXmlEventReader() throws XMLStreamException {
+        var factory = spy(new SecureXmlParserFactory());
+
+        assertThat(factory.createXmlEventReader(createEmptyXmlReader())).isNotNull();
+
+        var brokenXmlInputFactory = createBrokenXmlInputFactory();
+        when(factory.createXmlInputFactory()).thenReturn(brokenXmlInputFactory);
+
+        assertThatIllegalArgumentException().isThrownBy(() -> factory.createXmlEventReader(createEmptyXmlReader()))
+                .withMessage("Can't create instance of XMLEventReader");
+    }
+
     private XMLInputFactory createBrokenXmlInputFactory() throws XMLStreamException {
         var xmlInputFactory = mock(XMLInputFactory.class);
         when(xmlInputFactory.createXMLStreamReader((Reader) any())).thenThrow(new XMLStreamException(EXPECTED_EXCEPTION));
+        when(xmlInputFactory.createXMLEventReader((Reader) any())).thenThrow(new XMLStreamException(EXPECTED_EXCEPTION));
         return xmlInputFactory;
     }