Merge pull request #2907 from coheigea/coheigea/saxparserfactory

Adding XXE rules for SAXParserFactory
semgrep · May 17, 2023 · cfc6872 · cfc6872
2 parents fe553ed + e08b7cc
commit cfc6872
Show file tree

Hide file tree

Showing 8 changed files with 377 additions and 3 deletions.
diff --git a/java/lang/security/audit/xxe/documentbuilderfactory-disallow-doctype-decl-false.java b/java/lang/security/audit/xxe/documentbuilderfactory-disallow-doctype-decl-false.java
@@ -44,6 +44,12 @@ public void GoodXMLInputFactory4() throws  ParserConfigurationException {
         dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
     }
 
+    public void GoodSAXParserFactory() throws  ParserConfigurationException {
+        SAXParserFactory spf = SAXParserFactory.newInstance();
+        //ok:documentbuilderfactory-disallow-doctype-decl-false
+        spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+    }
+
 }
 
 class BadDocumentBuilderFactory{
@@ -55,3 +61,13 @@ public void BadXMLInputFactory() throws  ParserConfigurationException {
         //dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
     }
 }
+
+class BadSAXParserFactory{
+    public void BadSAXParserFactory() throws  ParserConfigurationException {
+        SAXParserFactory spf = SAXParserFactory.newInstance();
+        //ruleid:documentbuilderfactory-disallow-doctype-decl-false
+        spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", false);
+        //fix:documentbuilderfactory-disallow-doctype-decl-false
+        //spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+    }
+}
diff --git a/java/lang/security/audit/xxe/documentbuilderfactory-disallow-doctype-decl-false.yaml b/java/lang/security/audit/xxe/documentbuilderfactory-disallow-doctype-decl-false.yaml
@@ -29,7 +29,7 @@ rules:
     impact: HIGH
     confidence: HIGH
   message: >-
-    DOCTYPE declarations are enabled for this DocumentBuilderFactory.
+    DOCTYPE declarations are enabled for $DBFACTORY.
     Without prohibiting external entity declarations, this is vulnerable to XML external entity attacks.
     Disable this by setting the feature "http://apache.org/xml/features/disallow-doctype-decl" to true.
     Alternatively, allow DOCTYPE declarations and only prohibit external entities declarations.

diff --git a/java/lang/security/audit/xxe/documentbuilderfactory-external-general-entities-true.java b/java/lang/security/audit/xxe/documentbuilderfactory-external-general-entities-true.java
@@ -20,3 +20,19 @@ public void BadXMLInputFactory() throws  ParserConfigurationException {
         dbf.setFeature("http://xml.org/sax/features/external-general-entities" , true);
     }
 }
+
+class GoodSAXParserFactory {
+    public void GoodSAXParserFactory() throws  ParserConfigurationException {
+        SAXParserFactory spf = SAXParserFactory.newInstance();
+        //ok:documentbuilderfactory-external-general-entities-true
+        spf.setFeature("http://xml.org/sax/features/external-general-entities" , false);
+    }
+}
+
+class BadSAXParserFactory{
+    public void BadSAXParserFactory() throws  ParserConfigurationException {
+        SAXParserFactory spf = SAXParserFactory.newInstance();
+        //ruleid:documentbuilderfactory-external-general-entities-true
+        spf.setFeature("http://xml.org/sax/features/external-general-entities" , true);
+    }
+}
diff --git a/java/lang/security/audit/xxe/documentbuilderfactory-external-general-entities-true.yaml b/java/lang/security/audit/xxe/documentbuilderfactory-external-general-entities-true.yaml
@@ -28,7 +28,7 @@ rules:
     impact: HIGH
     confidence: HIGH
   message: >-
-    External entities are allowed for this DocumentBuilderFactory.
+    External entities are allowed for $DBFACTORY.
     This is vulnerable to XML external entity attacks. Disable this by setting the feature "http://xml.org/sax/features/external-general-entities"
     to false.
   pattern: $DBFACTORY.setFeature("http://xml.org/sax/features/external-general-entities", true);

diff --git a/java/lang/security/audit/xxe/documentbuilderfactory-external-parameter-entities-true.java b/java/lang/security/audit/xxe/documentbuilderfactory-external-parameter-entities-true.java
@@ -20,3 +20,19 @@ public void BadXMLInputFactory() throws  ParserConfigurationException {
         dbf.setFeature("http://xml.org/sax/features/external-parameter-entities" , true);
     }
 }
+
+class GoodSAXParserFactory {
+    public void GoodSAXParserFactory() throws  ParserConfigurationException {
+        SAXParserFactory spf = SAXParserFactory.newInstance();
+        //ok:documentbuilderfactory-external-parameter-entities-true
+        spf.setFeature("http://xml.org/sax/features/external-parameter-entities" , false);
+    }
+}
+
+class BadSAXParserFactory{
+    public void BadSAXParserFactory() throws  ParserConfigurationException {
+        SAXParserFactory spf = SAXParserFactory.newInstance();
+        //ruleid:documentbuilderfactory-external-parameter-entities-true
+        spf.setFeature("http://xml.org/sax/features/external-parameter-entities" , true);
+    }
+}
diff --git a/java/lang/security/audit/xxe/documentbuilderfactory-external-parameter-entities-true.yaml b/java/lang/security/audit/xxe/documentbuilderfactory-external-parameter-entities-true.yaml
@@ -28,7 +28,7 @@ rules:
     impact: HIGH
     confidence: HIGH
   message: >-
-    External entities are allowed for this DocumentBuilderFactory.
+    External entities are allowed for $DBFACTORY.
     This is vulnerable to XML external entity attacks. Disable this by setting the feature "http://xml.org/sax/features/external-parameter-entities"
     to false.
   pattern: $DBFACTORY.setFeature("http://xml.org/sax/features/external-parameter-entities", true);

diff --git a/java/lang/security/audit/xxe/saxparserfactory-disallow-doctype-decl-missing.java b/java/lang/security/audit/xxe/saxparserfactory-disallow-doctype-decl-missing.java
@@ -0,0 +1,165 @@
+package example;
+
+import javax.xml.parsers.SAXParserFactory;
+import javax.xml.parsers.SAXParser;
+import javax.xml.parsers.ParserConfigurationException;
+
+
+class GoodSAXParserFactory {
+    public void GoodSAXParserFactory() throws  ParserConfigurationException {
+        SAXParserFactory spf = SAXParserFactory.newInstance();
+        spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+        //ok:saxparserfactory-disallow-doctype-decl-missing
+        spf.newSAXParser();
+    }
+
+    public void GoodSAXParserFactory2() throws  ParserConfigurationException {
+        SAXParserFactory spf = SAXParserFactory.newInstance();
+        spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+        spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
+        //ok:saxparserfactory-disallow-doctype-decl-missing
+        spf.newSAXParser();
+    }
+
+    public void GoodSAXParserFactory3() throws  ParserConfigurationException {
+        SAXParserFactory spf = SAXParserFactory.newInstance();
+        spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
+        spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+        //ok:saxparserfactory-disallow-doctype-decl-missing
+        spf.newSAXParser();
+    }
+
+    public void GoodSAXParserFactory4() throws  ParserConfigurationException {
+        SAXParserFactory factory = XmlUtils.getSecureSAXParserFactory();
+        //Deep semgrep could find issues like this
+        //ok:saxparserfactory-disallow-doctype-decl-missing
+        saxparser = factory.newSAXParser();
+    }
+}
+
+class BadSAXParserFactory{
+    public void BadSAXParserFactory() throws  ParserConfigurationException {
+        SAXParserFactory spf = SAXParserFactory.newInstance();
+        //ruleid:saxparserfactory-disallow-doctype-decl-missing
+        spf.newSAXParser();
+    }
+
+    public void BadSAXParserFactory2() throws  ParserConfigurationException {
+        SAXParserFactory spf = SAXParserFactory.newInstance();
+        spf.setFeature("somethingElse", true);
+        //ruleid:saxparserfactory-disallow-doctype-decl-missing
+        spf.newSAXParser();
+    }
+}
+
+class GoodSAXParserFactoryStatic {
+
+    private static SAXParserFactory spf = SAXParserFactory.newInstance();
+
+    static {
+        spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+        //ok:saxparserfactory-disallow-doctype-decl-missing
+        spf.newSAXParser();
+    }
+
+}
+
+class BadSAXParserFactoryStatic {
+
+    private static SAXParserFactory spf = SAXParserFactory.newInstance();
+
+    static {
+        spf.setFeature("not-a-secure-feature", true);
+    }
+
+    public void doSomething(){
+        //ruleid:saxparserfactory-disallow-doctype-decl-missing
+        spf.newSAXParser();
+    }
+
+}
+
+class OneMoreGoodSAXParserFactory {
+
+    public void GoodSAXParserFactory(boolean condition) throws  ParserConfigurationException {
+        SAXParserFactory spf = null;
+
+        if ( condition ) {
+            spf = SAXParserFactor.newInstance();
+        } else {
+            spf = newFactory();
+        }
+        spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+        //ok:saxparserfactory-disallow-doctype-decl-missing
+        spf.newSAXParser();
+    }
+
+    private SAXParserFactory newFactory(){
+        return SAXParserFactory.newInstance();
+    }
+
+}
+
+class OneMoreBadSAXParserFactory {
+
+    public void GoodSAXParserFactory(boolean condition) throws  ParserConfigurationException {
+        SAXParserFactory spf = null;
+
+        if ( condition ) {
+            spf = SAXParserFactory.newInstance();
+        } else {
+            spf = newFactory();
+        }
+        //ruleid:saxparserfactory-disallow-doctype-decl-missing
+        spf.newSAXParser();
+    }
+
+    private SAXParserFactory newFactory(){
+        return SAXParserFactory.newInstance();
+    }
+
+
+}
+
+
+class GoodSAXParserFactoryCtr {
+
+    private final SAXParserFactory spf;
+
+    public GoodSAXParserFactoryCtr() throws Exception {
+        spf = SAXParserFactory.newInstance();
+        spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+        //ok:saxparserfactory-disallow-doctype-decl-missing
+        spf.newSAXParser();
+    }
+}
+
+
+class GoodSAXParserFactoryCtr2 {
+    public void somemethod() throws Exception {
+        SAXParserFactory spf = SAXParserFactory.newInstance();
+        setFeatures(spf);
+        //ok:saxparserfactory-disallow-doctype-decl-missing
+        spf.newSAXParser();
+    }
+
+    private void setFeatures(SAXParserFactory spf) throws Exception {
+        spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+    }
+
+}
+
+class GoodSAXParserFactoryCtr3 {
+    public void somemethod() throws Exception {
+        SAXParserFactory spf = SAXParserFactory.newInstance();
+        setFeatures(spf);
+        //ok:saxparserfactory-disallow-doctype-decl-missing
+        spf.newSAXParser();
+    }
+
+    private void setFeatures(SAXParserFactory spf) throws Exception {
+        spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
+        spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+    }
+
+}