protect MavenReader against XXE vulnerability

ohecker · Mar 26, 2024 · 70f502a · 70f502a
1 parent 93f23d8
commit 70f502a
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 3 deletions.
diff --git a/core/src/main/java/com/devonfw/tools/solicitor/reader/maven/MavenReader.java b/core/src/main/java/com/devonfw/tools/solicitor/reader/maven/MavenReader.java
@@ -10,11 +10,20 @@
 import java.util.Map;
 import java.util.Set;
 
+import javax.xml.XMLConstants;
 import javax.xml.bind.JAXBContext;
 import javax.xml.bind.JAXBException;
 import javax.xml.bind.Unmarshaller;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+import javax.xml.transform.Source;
+import javax.xml.transform.sax.SAXSource;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.stereotype.Component;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
 
 import com.devonfw.tools.solicitor.common.PackageURLHelper;
 import com.devonfw.tools.solicitor.common.SolicitorRuntimeException;
@@ -33,6 +42,7 @@
  */
 @Component
 public class MavenReader extends AbstractReader implements Reader {
+  private static final Logger LOG = LoggerFactory.getLogger(MavenReader.class);
 
   /**
    * The supported type of this {@link Reader}.
@@ -63,11 +73,26 @@ public void readInventory(String type, String sourceUrl, Application application
 
     JAXBContext jaxbContext;
     try {
+      SAXParserFactory secureParserFactory = SAXParserFactory.newInstance();
+      secureParserFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+      secureParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+      secureParserFactory.setXIncludeAware(false);
+
+      Source xmlSource = new SAXSource(secureParserFactory.newSAXParser().getXMLReader(), new InputSource(is));
+
       jaxbContext = JAXBContext.newInstance(LicenseSummary.class);
       Unmarshaller unmarshaller = jaxbContext.createUnmarshaller();
-      ls = (LicenseSummary) unmarshaller.unmarshal(is);
-    } catch (JAXBException e) {
-      throw new SolicitorRuntimeException("Could nor read maven license info", e);
+      ls = (LicenseSummary) unmarshaller.unmarshal(xmlSource);
+    } catch (JAXBException | SAXException | ParserConfigurationException e) {
+      throw new SolicitorRuntimeException("Could not read maven license info", e);
+    } finally {
+      if (is != null) {
+        try {
+          is.close();
+        } catch (IOException e) {
+          LOG.debug("Exception while attemping to close inputs stream for reading maven license data", e);
+        }
+      }
     }
 
     for (Dependency dep : ls.getDependencies()) {

diff --git a/core/src/test/java/com/devonfw/tools/solicitor/reader/maven/MavenReaderTests.java b/core/src/test/java/com/devonfw/tools/solicitor/reader/maven/MavenReaderTests.java
@@ -6,12 +6,16 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import javax.xml.bind.UnmarshalException;
 
 import org.junit.jupiter.api.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.devonfw.tools.solicitor.common.FileInputStreamFactory;
+import com.devonfw.tools.solicitor.common.SolicitorRuntimeException;
 import com.devonfw.tools.solicitor.model.ModelFactory;
 import com.devonfw.tools.solicitor.model.impl.ModelFactoryImpl;
 import com.devonfw.tools.solicitor.model.inventory.ApplicationComponent;
@@ -21,6 +25,9 @@
 public class MavenReaderTests {
   private static final Logger LOG = LoggerFactory.getLogger(MavenReaderTests.class);
 
+  /**
+   * Tests reading a maven license file.
+   */
   @Test
   public void readFileAndCheckSize() {
 
@@ -50,4 +57,30 @@ public void readFileAndCheckSize() {
     assertTrue(found);
 
   }
+
+  /**
+   * Tests if the MavenReader rejects XML with DOCTYPE declaration which is done to prevent XXE.
+   */
+  @Test
+  public void testProtectionAgainstXxe() {
+
+    ModelFactory modelFactory = new ModelFactoryImpl();
+
+    Application application = modelFactory.newApplication("testApp", "0.0.0.TEST", "1.1.2111", "http://bla.com",
+        "Java8");
+    MavenReader mr = new MavenReader();
+    mr.setModelFactory(modelFactory);
+    mr.setInputStreamFactory(new FileInputStreamFactory());
+
+    try {
+      mr.readInventory("maven", "src/test/resources/licenses_sample_with_doctype.xml", application,
+          UsagePattern.DYNAMIC_LINKING, "maven", null);
+      fail("Expected exception was not thrown");
+    } catch (SolicitorRuntimeException e) {
+      // we check detailed message to make sure the exception is not thrown due to other reasons
+      UnmarshalException ume = (UnmarshalException) (e.getCause());
+      assertTrue(ume.getLinkedException().getMessage().contains("DOCTYPE is disallowed"));
+    }
+
+  }
 }
diff --git a/core/src/test/resources/licenses_sample_with_doctype.xml b/core/src/test/resources/licenses_sample_with_doctype.xml
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<!DOCTYPE licenseSummary [<!ELEMENT licenseSummary ANY >]>
+<licenseSummary>
+  <dependencies>
+    <dependency>
+      <groupId>org.apache.poi</groupId>
+      <artifactId>poi-ooxml</artifactId>
+      <version>3.17</version>
+      <licenses>
+        <license>
+          <name>The Apache Software License, Version 2.0</name>
+          <url>http://www.apache.org/licenses/LICENSE-2.0.txt</url>
+        </license>
+      </licenses>
+    </dependency>
+  </dependencies>
+</licenseSummary>