From 4dd6dedaa4d4a731d02b6ed2c9ed6450fcc43891 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sat, 3 Dec 2016 17:44:49 -0500 Subject: [PATCH] hardening the XML parser per jacks.codiscope.com --- .../dependencycheck/analyzer/AssemblyAnalyzer.java | 10 +++++++++- .../dependencycheck/data/central/CentralSearch.java | 3 +++ .../owasp/dependencycheck/data/nexus/NexusSearch.java | 10 +++++++++- .../dependencycheck/data/nuget/XPathNuspecParser.java | 3 +++ .../owasp/dependencycheck/data/update/CpeUpdater.java | 3 +++ .../dependencycheck/data/update/nvd/ProcessTask.java | 3 +++ .../owasp/dependencycheck/xml/hints/HintParser.java | 3 +++ .../org/owasp/dependencycheck/xml/pom/PomParser.java | 3 +++ .../xml/suppression/SuppressionParser.java | 6 ++++++ 9 files changed, 42 insertions(+), 2 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java index 872e1484b..cd9551006 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java @@ -43,6 +43,7 @@ import javax.xml.xpath.XPathExpressionException; import javax.xml.xpath.XPathFactory; import java.util.ArrayList; import java.util.List; +import javax.xml.XMLConstants; import javax.xml.parsers.ParserConfigurationException; import org.owasp.dependencycheck.exception.InitializationException; import org.apache.commons.lang3.SystemUtils; @@ -124,7 +125,12 @@ public class AssemblyAnalyzer extends AbstractFileTypeAnalyzer { try { final Process proc = pb.start(); - final DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); + final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + final DocumentBuilder builder = factory.newDocumentBuilder(); + doc = builder.parse(proc.getInputStream()); // Try evacuating the error stream @@ -261,6 +267,8 @@ public class AssemblyAnalyzer extends AbstractFileTypeAnalyzer { final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); final DocumentBuilder builder = factory.newDocumentBuilder(); final Document doc = builder.parse(p.getInputStream()); final XPath xpath = XPathFactory.newInstance().newXPath(); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/central/CentralSearch.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/central/CentralSearch.java index 6571e4d8c..068d167d2 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/central/CentralSearch.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/central/CentralSearch.java @@ -23,6 +23,7 @@ import java.net.HttpURLConnection; import java.net.URL; import java.util.ArrayList; import java.util.List; +import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.xpath.XPath; @@ -112,6 +113,8 @@ public class CentralSearch { try { final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); final DocumentBuilder builder = factory.newDocumentBuilder(); final Document doc = builder.parse(conn.getInputStream()); final XPath xpath = XPathFactory.newInstance().newXPath(); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nexus/NexusSearch.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nexus/NexusSearch.java index 71c52d466..88ed54326 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nexus/NexusSearch.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nexus/NexusSearch.java @@ -21,6 +21,7 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.net.HttpURLConnection; import java.net.URL; +import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.xpath.XPath; @@ -106,6 +107,8 @@ public class NexusSearch { try { final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); final DocumentBuilder builder = factory.newDocumentBuilder(); final Document doc = builder.parse(conn.getInputStream()); final XPath xpath = XPathFactory.newInstance().newXPath(); @@ -167,7 +170,12 @@ public class NexusSearch { LOGGER.warn("Expected 200 result from Nexus, got {}", conn.getResponseCode()); return false; } - final DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); + final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + final DocumentBuilder builder = factory.newDocumentBuilder(); + final Document doc = builder.parse(conn.getInputStream()); if (!"status".equals(doc.getDocumentElement().getNodeName())) { LOGGER.warn("Expected root node name of status, got {}", doc.getDocumentElement().getNodeName()); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nuget/XPathNuspecParser.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nuget/XPathNuspecParser.java index 2769a17e7..0ce80df92 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nuget/XPathNuspecParser.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nuget/XPathNuspecParser.java @@ -18,6 +18,7 @@ package org.owasp.dependencycheck.data.nuget; import java.io.InputStream; +import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.xpath.XPath; import javax.xml.xpath.XPathConstants; @@ -59,6 +60,8 @@ public class XPathNuspecParser implements NuspecParser { try { final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); final Document d = factory.newDocumentBuilder().parse(stream); final XPath xpath = XPathFactory.newInstance().newXPath(); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/CpeUpdater.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/CpeUpdater.java index 69ec4bcdf..1ee134eb2 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/CpeUpdater.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/CpeUpdater.java @@ -26,6 +26,7 @@ import java.net.MalformedURLException; import java.net.URL; import java.util.List; import java.util.zip.GZIPInputStream; +import javax.xml.XMLConstants; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; @@ -123,6 +124,8 @@ public class CpeUpdater extends BaseUpdater implements CachedWebDataSource { try { final SAXParserFactory factory = SAXParserFactory.newInstance(); factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); final SAXParser saxParser = factory.newSAXParser(); final CPEHandler handler = new CPEHandler(); saxParser.parse(xml, handler); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/nvd/ProcessTask.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/nvd/ProcessTask.java index aaa89a91b..cffeba5e4 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/nvd/ProcessTask.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/nvd/ProcessTask.java @@ -24,6 +24,7 @@ import java.sql.SQLException; import java.util.List; import java.util.Map; import java.util.concurrent.Callable; +import javax.xml.XMLConstants; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; @@ -144,6 +145,8 @@ public class ProcessTask implements Callable { final SAXParserFactory factory = SAXParserFactory.newInstance(); factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); final SAXParser saxParser = factory.newSAXParser(); final NvdCve12Handler cve12Handler = new NvdCve12Handler(); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/hints/HintParser.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/hints/HintParser.java index 15a4a0d5a..e5515a1c6 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/hints/HintParser.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/hints/HintParser.java @@ -24,6 +24,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.Reader; +import javax.xml.XMLConstants; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; @@ -110,6 +111,8 @@ public class HintParser { final HintHandler handler = new HintHandler(); final SAXParserFactory factory = SAXParserFactory.newInstance(); factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); factory.setNamespaceAware(true); factory.setValidating(true); final SAXParser saxParser = factory.newSAXParser(); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/pom/PomParser.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/pom/PomParser.java index aef0899ad..78734d94e 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/pom/PomParser.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/pom/PomParser.java @@ -24,6 +24,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.Reader; +import javax.xml.XMLConstants; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; @@ -89,6 +90,8 @@ public class PomParser { // factory.setNamespaceAware(true); // factory.setValidating(true); factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); final SAXParser saxParser = factory.newSAXParser(); final XMLReader xmlReader = saxParser.getXMLReader(); xmlReader.setContentHandler(handler); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionParser.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionParser.java index 5ddf49c4d..b4a7fc30c 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionParser.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionParser.java @@ -25,6 +25,7 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.io.Reader; import java.util.List; +import javax.xml.XMLConstants; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; @@ -129,6 +130,8 @@ public class SuppressionParser { factory.setNamespaceAware(true); factory.setValidating(true); factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); final SAXParser saxParser = factory.newSAXParser(); saxParser.setProperty(SuppressionParser.JAXP_SCHEMA_LANGUAGE, SuppressionParser.W3C_XML_SCHEMA); saxParser.setProperty(SuppressionParser.JAXP_SCHEMA_SOURCE, new InputSource(schemaStream)); @@ -184,6 +187,9 @@ public class SuppressionParser { schemaStream = this.getClass().getClassLoader().getResourceAsStream(OLD_SUPPRESSION_SCHEMA); final SuppressionHandler handler = new SuppressionHandler(); final SAXParserFactory factory = SAXParserFactory.newInstance(); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); factory.setNamespaceAware(true); factory.setValidating(true); final SAXParser saxParser = factory.newSAXParser();