Fix Java xmlinputfactory rules (semgrep#3347)

* fix java xmlinputfactory rules * fix java xmlinputfactory rules * fix java xmlinputfactory rules
Starkteetje · Apr 8, 2024 · 4b0ac74 · 4b0ac74
1 parent 144ad6e
commit 4b0ac74
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 12 deletions.
diff --git a/java/lang/security/xmlinputfactory-external-entities-enabled.java b/java/lang/security/xmlinputfactory-external-entities-enabled.java
@@ -3,6 +3,7 @@
 package example;
 
 import javax.xml.stream.XMLInputFactory;
+import static javax.xml.stream.XMLInputFactory.SUPPORT_DTD;
 
 class GoodXMLInputFactory {
     public GoodXMLInputFactory() {
@@ -16,10 +17,31 @@ public GoodXMLInputFactory() {
     }
 }
 
+class GoodXMLInputFactory1 {
+    public GoodXMLInputFactory1() {
+        final XMLInputFactory xmlInputFactory = XMLInputFactory.newFactory();
+
+        // See
+        // https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.md#xmlinputfactory-a-stax-parser
+        // ok:xmlinputfactory-external-entities-enabled
+        xmlInputFactory.setProperty(SUPPORT_DTD, false);
+    }
+}
+
 class BadXMLInputFactory {
     public BadXMLInputFactory() {
         final XMLInputFactory xmlInputFactory = XMLInputFactory.newFactory();
         // ruleid:xmlinputfactory-external-entities-enabled
         xmlInputFactory.setProperty("javax.xml.stream.isSupportingExternalEntities", true);
     }
 }
+
+class BadXMLInputFactory1 {
+    public BadXMLInputFactory1() {
+        final XMLInputFactory xmlInputFactory = XMLInputFactory.newFactory();
+        // ruleid:xmlinputfactory-external-entities-enabled
+        xmlInputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, true);
+    }
+}
+
+
diff --git a/java/lang/security/xmlinputfactory-external-entities-enabled.yaml b/java/lang/security/xmlinputfactory-external-entities-enabled.yaml
@@ -31,6 +31,9 @@ rules:
     to XML external entity
     attacks. Disable external entities by setting "javax.xml.stream.isSupportingExternalEntities"
     to false.
-  pattern: $XMLFACTORY.setProperty("javax.xml.stream.isSupportingExternalEntities", true);
+  patterns:
+  - pattern-either:
+      - pattern: (javax.xml.stream.XMLInputFactory $XMLFACTORY).setProperty("javax.xml.stream.isSupportingExternalEntities", true);
+      - pattern: (javax.xml.stream.XMLInputFactory $XMLFACTORY).setProperty(javax.xml.stream.XMLInputFactory.SUPPORT_DTD, true);
   languages:
   - java
diff --git a/java/lang/security/xmlinputfactory-possible-xxe.java b/java/lang/security/xmlinputfactory-possible-xxe.java
@@ -3,10 +3,10 @@
 package example;
 
 import javax.xml.stream.XMLInputFactory;
-import static java.xml.stream.XMLFactoryInput.IS_SUPPORTING_EXTERNAL_ENTITIES;
+import static javax.xml.stream.XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES;
 
 class GoodXMLInputFactory {
-    public void Blah() {
+    public void blah() {
         final XMLInputFactory xmlInputFactory = XMLInputFactory.newFactory();
 
         // See
@@ -18,7 +18,7 @@ public void Blah() {
 }
 
 class GoodConstXMLInputFactory {
-    public void Blah() {
+    public GoodConstXMLInputFactory() {
         final XMLInputFactory xmlInputFactory = XMLInputFactory.newFactory();
 
         // See
@@ -29,16 +29,24 @@ public void Blah() {
     }
 }
 
-class BadXMLInputFactory {
-    public Blah() {
+class BadXMLInputFactory1 {
+    public BadXMLInputFactory1() {
         // ruleid:xmlinputfactory-possible-xxe
         final XMLInputFactory xmlInputFactory = XMLInputFactory.newFactory();
         xmlInputFactory.setProperty("javax.xml.stream.isSupportingExternalEntities", true);
     }
 }
 
+class BadXMLInputFactory2 {
+    public BadXMLInputFactory2() {
+        // ruleid:xmlinputfactory-possible-xxe
+        final XMLInputFactory xmlInputFactory = XMLInputFactory.newFactory();
+        xmlInputFactory.setProperty(IS_SUPPORTING_EXTERNAL_ENTITIES, true);
+    }
+}
+
 class MaybeBadXMLInputFactory {
-    public Blah() {
+    public void foobar() {
         // ruleid:xmlinputfactory-possible-xxe
         final XMLInputFactory xmlInputFactory = XMLInputFactory.newFactory();
     }

diff --git a/java/lang/security/xmlinputfactory-possible-xxe.yaml b/java/lang/security/xmlinputfactory-possible-xxe.yaml
@@ -34,19 +34,19 @@ rules:
     to false.
   patterns:
   - pattern-not-inside: |
-      $RETURNTYPE $METHOD(...) {
+      $METHOD(...) {
         ...
         $XMLFACTORY.setProperty("javax.xml.stream.isSupportingExternalEntities", false);
         ...
       }
   - pattern-not-inside: |
-      $RETURNTYPE $METHOD(...) {
+      $METHOD(...) {
         ...
-        $XMLFACTORY.setProperty(java.xml.stream.XMLFactoryInput.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+        $XMLFACTORY.setProperty(javax.xml.stream.XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
         ...
       }
   - pattern-either:
-    - pattern: $XMLFACTORY = $W.newFactory(...);
-    - pattern: $XMLFACTORY = new XMLInputFactory(...);
+    - pattern: javax.xml.stream.XMLInputFactory.newFactory(...)
+    - pattern: new XMLInputFactory(...)
   languages:
   - java