add new additional taint steps, fix some comments

am0o0 · am0o0 · commit 4b68dd231590 · 2024-05-12T13:51:08.000+02:00
diff --git a/java/ql/src/experimental/Security/CWE/CWE-522-DecompressionBombs/DecompressionBomb.ql b/java/ql/src/experimental/Security/CWE/CWE-522-DecompressionBombs/DecompressionBomb.ql
@@ -27,11 +27,11 @@ module DecompressionBombsConfig implements DataFlow::StateConfigSig {
       or
       source instanceof FileUploadRemoteFlowSource
     ) and
-    state = ["ZipFile", "Zip4j", "inflator", "Zip", "ApacheCommons", "XerialSnappy"]
+    state = ["ZipFile", "Zip4j", "inflator", "UtilZip", "ApacheCommons", "XerialSnappy"]
   }
 
   predicate isSink(DataFlow::Node sink, FlowState state) {
-    any(DecompressionBomb::Sink ads).sink(sink, state)
+    any(DecompressionBomb::Sink s).sink(sink, state)
   }
 
   predicate isAdditionalFlowStep(
diff --git a/java/ql/src/experimental/semmle/code/java/security/DecompressionBomb.qll b/java/ql/src/experimental/semmle/code/java/security/DecompressionBomb.qll
@@ -38,7 +38,7 @@ module XerialSnappy {
   }
 
   /**
-   * Gets `n1` and `n2` which `SnappyInputStream n2 = new SnappyInputStream(n2)` or
+   * Gets `n1` and `n2` which `SnappyInputStream n2 = new SnappyInputStream(n1)` or
    * `n1.read(n2)`,
    *  second one is added because of sanitizer, we want to compare return value of each `read` or similar method
    *  that whether there is a flow to a comparison between total read of decompressed stream and a constant value
@@ -482,7 +482,7 @@ module CommonsIO {
   class Sink extends DecompressionBomb::Sink {
     override predicate sink(DataFlow::Node sink, DataFlow::FlowState state) {
       sink.asExpr() = any(IOUtils r).getArgument(0) and
-      state = ["ZipFile", "Zip4j", "inflator", "Zip", "ApacheCommons", "XerialSnappy"]
+      state = ["ZipFile", "Zip4j", "inflator", "UtilZip", "ApacheCommons", "XerialSnappy"]
     }
   }
 }
@@ -522,7 +522,7 @@ module Zip {
 
   class ReadInputStreamSink extends DecompressionBomb::Sink {
     override predicate sink(DataFlow::Node sink, DataFlow::FlowState state) {
-      sink.asExpr() = any(ReadInputStreamCall r).getAByteRead() and state = "Zip"
+      sink.asExpr() = any(ReadInputStreamCall r).getAByteRead() and state = "UtilZip"
     }
   }
 
@@ -549,8 +549,8 @@ module Zip {
         call.getQualifier() = n1.asExpr() and
         call = n2.asExpr()
       ) and
-      stateFrom = "Zip" and
-      stateTo = "Zip"
+      stateFrom = "UtilZip" and
+      stateTo = "UtilZip"
     }
   }
 
@@ -708,6 +708,24 @@ module InputStream {
       ) and
       stateFrom = "ZipFile" and
       stateTo = "ZipFile"
+      or
+      exists(Call call |
+        // Method calls
+        call.(ConstructorCall).getConstructedType().hasQualifiedName("java.util.zip", "ZipFile") and
+        n1.asExpr() = call.getAnArgument() and
+        n2.asExpr() = call
+      ) and
+      stateFrom = "ZipFile" and
+      stateTo = "ZipFile"
     }
   }
 }
+
+predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+  exists(Call call |
+    // Method calls
+    call.(ConstructorCall).getConstructedType().hasQualifiedName("java.util.zip", "ZipFile") and
+    n1.asExpr() = call.getAnArgument() and
+    n2.asExpr() = call
+  )
+}

Original file line number	Diff line number	Diff line change
`@@ -27,11 +27,11 @@ module DecompressionBombsConfig implements DataFlow::StateConfigSig {`
`27`	`27`	`or`
`28`	`28`	`source instanceof FileUploadRemoteFlowSource`
`29`	`29`	`) and`
`30`		`- state = ["ZipFile", "Zip4j", "inflator", "Zip", "ApacheCommons", "XerialSnappy"]`
	`30`	`+ state = ["ZipFile", "Zip4j", "inflator", "UtilZip", "ApacheCommons", "XerialSnappy"]`
`31`	`31`	`}`
`32`	`32`
`33`	`33`	`predicate isSink(DataFlow::Node sink, FlowState state) {`
`34`		`- any(DecompressionBomb::Sink ads).sink(sink, state)`
	`34`	`+ any(DecompressionBomb::Sink s).sink(sink, state)`
`35`	`35`	`}`
`36`	`36`
`37`	`37`	`predicate isAdditionalFlowStep(`
Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ module XerialSnappy {`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`/**`
`41`		- * Gets `n1` and `n2` which `SnappyInputStream n2 = new SnappyInputStream(n2)` or
	`41`	+ * Gets `n1` and `n2` which `SnappyInputStream n2 = new SnappyInputStream(n1)` or
`42`	`42`	* `n1.read(n2)`,
`43`	`43`	* second one is added because of sanitizer, we want to compare return value of each `read` or similar method
`44`	`44`	`* that whether there is a flow to a comparison between total read of decompressed stream and a constant value`
`@@ -482,7 +482,7 @@ module CommonsIO {`
`482`	`482`	`class Sink extends DecompressionBomb::Sink {`
`483`	`483`	`override predicate sink(DataFlow::Node sink, DataFlow::FlowState state) {`
`484`	`484`	`sink.asExpr() = any(IOUtils r).getArgument(0) and`
`485`		`- state = ["ZipFile", "Zip4j", "inflator", "Zip", "ApacheCommons", "XerialSnappy"]`
	`485`	`+ state = ["ZipFile", "Zip4j", "inflator", "UtilZip", "ApacheCommons", "XerialSnappy"]`
`486`	`486`	`}`
`487`	`487`	`}`
`488`	`488`	`}`
`@@ -522,7 +522,7 @@ module Zip {`
`522`	`522`
`523`	`523`	`class ReadInputStreamSink extends DecompressionBomb::Sink {`
`524`	`524`	`override predicate sink(DataFlow::Node sink, DataFlow::FlowState state) {`
`525`		`- sink.asExpr() = any(ReadInputStreamCall r).getAByteRead() and state = "Zip"`
	`525`	`+ sink.asExpr() = any(ReadInputStreamCall r).getAByteRead() and state = "UtilZip"`
`526`	`526`	`}`
`527`	`527`	`}`
`528`	`528`
`@@ -549,8 +549,8 @@ module Zip {`
`549`	`549`	`call.getQualifier() = n1.asExpr() and`
`550`	`550`	`call = n2.asExpr()`
`551`	`551`	`) and`
`552`		`- stateFrom = "Zip" and`
`553`		`- stateTo = "Zip"`
	`552`	`+ stateFrom = "UtilZip" and`
	`553`	`+ stateTo = "UtilZip"`
`554`	`554`	`}`
`555`	`555`	`}`
`556`	`556`
`@@ -708,6 +708,24 @@ module InputStream {`
`708`	`708`	`) and`
`709`	`709`	`stateFrom = "ZipFile" and`
`710`	`710`	`stateTo = "ZipFile"`
	`711`	`+ or`
	`712`	`+ exists(Call call \|`
	`713`	`+ // Method calls`
	`714`	`+ call.(ConstructorCall).getConstructedType().hasQualifiedName("java.util.zip", "ZipFile") and`
	`715`	`+ n1.asExpr() = call.getAnArgument() and`
	`716`	`+ n2.asExpr() = call`
	`717`	`+ ) and`
	`718`	`+ stateFrom = "ZipFile" and`
	`719`	`+ stateTo = "ZipFile"`
`711`	`720`	`}`
`712`	`721`	`}`
`713`	`722`	`}`
	`723`	`+`
	`724`	`+predicate step(DataFlow::Node n1, DataFlow::Node n2) {`
	`725`	`+ exists(Call call \|`
	`726`	`+ // Method calls`
	`727`	`+ call.(ConstructorCall).getConstructedType().hasQualifiedName("java.util.zip", "ZipFile") and`
	`728`	`+ n1.asExpr() = call.getAnArgument() and`
	`729`	`+ n2.asExpr() = call`
	`730`	`+ )`
	`731`	`+}`