Merge pull request #4799 from joefarebrother/xxe-fp

Java: Fix false positive in the XXE query

Author: yo-h

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.
+
+

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 |

java/ql/test/query-tests/security/CWE-611/SAXParserTests.java

@@ -2,7 +2,7 @@
 
 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 void misConfiguredParser3(Socket sock) throws Exception {
     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
+  }
 }