Python: Move experimental UnsafeUnpack to new dataflow API

RasmusWL · RasmusWL · commit 3cdd875e9fc7 · 2023-08-28T15:31:07.000+02:00
diff --git a/python/ql/src/experimental/Security/CWE-022bis/UnsafeUnpack.ql b/python/ql/src/experimental/Security/CWE-022bis/UnsafeUnpack.ql
@@ -16,9 +16,9 @@
 
 import python
 import experimental.Security.UnsafeUnpackQuery
-import DataFlow::PathGraph
+import UnsafeUnpackFlow::PathGraph
 
-from UnsafeUnpackingConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
-where config.hasFlowPath(source, sink)
+from UnsafeUnpackFlow::PathNode source, UnsafeUnpackFlow::PathNode sink
+where UnsafeUnpackFlow::flowPath(source, sink)
 select sink.getNode(), source, sink,
   "Unsafe extraction from a malicious tarball retrieved from a remote location."
diff --git a/python/ql/src/experimental/Security/UnsafeUnpackQuery.qll b/python/ql/src/experimental/Security/UnsafeUnpackQuery.qll
@@ -39,10 +39,8 @@ class AllTarfileOpens extends API::CallNode {
   }
 }
 
-class UnsafeUnpackingConfig extends TaintTracking::Configuration {
-  UnsafeUnpackingConfig() { this = "UnsafeUnpackingConfig" }
-
-  override predicate isSource(DataFlow::Node source) {
+private module UnsafeUnpackConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
     // A source coming from a remote location
     source instanceof RemoteFlowSource
     or
@@ -92,7 +90,7 @@ class UnsafeUnpackingConfig extends TaintTracking::Configuration {
     source.(AttrRead).getAttributeName() = "FILES"
   }
 
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     (
       // A sink capturing method calls to `unpack_archive`.
       sink = API::moduleImport("shutil").getMember("unpack_archive").getACall().getArg(0)
@@ -136,7 +134,7 @@ class UnsafeUnpackingConfig extends TaintTracking::Configuration {
     not sink.getScope().getLocation().getFile().inStdlib()
   }
 
-  override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+  predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
     // Reading the response
     nodeTo.(MethodCallNode).calls(nodeFrom, "read")
     or
@@ -211,3 +209,6 @@ class UnsafeUnpackingConfig extends TaintTracking::Configuration {
     )
   }
 }
+
+/** Global taint-tracking for detecting "UnsafeUnpacking" vulnerabilities. */
+module UnsafeUnpackFlow = TaintTracking::Global<UnsafeUnpackConfig>;
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-022-UnsafeUnpacking/UnsafeUnpack.expected b/python/ql/test/experimental/query-tests/Security/CWE-022-UnsafeUnpacking/UnsafeUnpack.expected

Original file line number	Diff line number	Diff line change
`@@ -39,10 +39,8 @@ class AllTarfileOpens extends API::CallNode {`
`39`	`39`	`}`
`40`	`40`	`}`
`41`	`41`
`42`		`-class UnsafeUnpackingConfig extends TaintTracking::Configuration {`
`43`		`- UnsafeUnpackingConfig() { this = "UnsafeUnpackingConfig" }`
`44`		`-`
`45`		`- override predicate isSource(DataFlow::Node source) {`
	`42`	`+private module UnsafeUnpackConfig implements DataFlow::ConfigSig {`
	`43`	`+ predicate isSource(DataFlow::Node source) {`
`46`	`44`	`// A source coming from a remote location`
`47`	`45`	`source instanceof RemoteFlowSource`
`48`	`46`	`or`
`@@ -92,7 +90,7 @@ class UnsafeUnpackingConfig extends TaintTracking::Configuration {`
`92`	`90`	`source.(AttrRead).getAttributeName() = "FILES"`
`93`	`91`	`}`
`94`	`92`
`95`		`- override predicate isSink(DataFlow::Node sink) {`
	`93`	`+ predicate isSink(DataFlow::Node sink) {`
`96`	`94`	`(`
`97`	`95`	// A sink capturing method calls to `unpack_archive`.
`98`	`96`	`sink = API::moduleImport("shutil").getMember("unpack_archive").getACall().getArg(0)`
`@@ -136,7 +134,7 @@ class UnsafeUnpackingConfig extends TaintTracking::Configuration {`
`136`	`134`	`not sink.getScope().getLocation().getFile().inStdlib()`
`137`	`135`	`}`
`138`	`136`
`139`		`- override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {`
	`137`	`+ predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {`
`140`	`138`	`// Reading the response`
`141`	`139`	`nodeTo.(MethodCallNode).calls(nodeFrom, "read")`
`142`	`140`	`or`
`@@ -211,3 +209,6 @@ class UnsafeUnpackingConfig extends TaintTracking::Configuration {`
`211`	`209`	`)`
`212`	`210`	`}`
`213`	`211`	`}`
	`212`	`+`
	`213`	`+/** Global taint-tracking for detecting "UnsafeUnpacking" vulnerabilities. */`
	`214`	`+module UnsafeUnpackFlow = TaintTracking::Global<UnsafeUnpackConfig>;`