diff --git a/java/change-notes/2020-12-09-xxe-fp-fix.md b/java/change-notes/2020-12-09-xxe-fp-fix.md new file mode 100644 index 00000000000..2ccf4bf369a --- /dev/null +++ b/java/change-notes/2020-12-09-xxe-fp-fix.md @@ -0,0 +1,4 @@ +lgtm,codescanning +* The query "Resolving XML external entity in user-controlled data" (`java/xxe`) has been improved to report fewer false positives when a `SAXParserFactory` is configured safely. + + diff --git a/java/ql/src/semmle/code/java/security/XmlParsers.qll b/java/ql/src/semmle/code/java/security/XmlParsers.qll index 7701af08923..685c5754fc9 100644 --- a/java/ql/src/semmle/code/java/security/XmlParsers.qll +++ b/java/ql/src/semmle/code/java/security/XmlParsers.qll @@ -481,6 +481,10 @@ class SAXParserFactoryConfig extends ParserConfig { class SafeSAXParserFactory extends VarAccess { SafeSAXParserFactory() { exists(Variable v | v = this.getVariable() | + exists(SAXParserFactoryConfig config | config.getQualifier() = v.getAnAccess() | + config.enables(singleSafeConfig()) + ) + or exists(SAXParserFactoryConfig config | config.getQualifier() = v.getAnAccess() | config .disables(any(ConstantStringExpr s | diff --git a/java/ql/test/query-tests/security/CWE-611/SAXParserTests.java b/java/ql/test/query-tests/security/CWE-611/SAXParserTests.java index 882cb79bac8..f8079dd1bc8 100644 --- a/java/ql/test/query-tests/security/CWE-611/SAXParserTests.java +++ b/java/ql/test/query-tests/security/CWE-611/SAXParserTests.java @@ -2,7 +2,7 @@ import java.net.Socket; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; - +import javax.xml.XMLConstants; import org.xml.sax.helpers.DefaultHandler; public class SAXParserTests { @@ -72,4 +72,12 @@ public class SAXParserTests { SAXParser parser = factory.newSAXParser(); parser.parse(sock.getInputStream(), new DefaultHandler()); //unsafe } + + public void safeParser2(Socket sock) throws Exception { + SAXParserFactory factory = SAXParserFactory.newInstance(); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + SAXParser parser = factory.newSAXParser(); + parser.parse(sock.getInputStream(), new DefaultHandler()); //safe + } }