python: update use of barrier guard

Author: yoff

python/ql/lib/semmle/python/security/dataflow/TarSlipCustomizations.qll

@@ -32,20 +32,22 @@ module TarSlip {
   abstract class Sanitizer extends DataFlow::Node { }
 
   /**
+   * DEPRECATED: Use `Sanitizer` instead.
+   *
    * A sanitizer guard for "tar slip" vulnerabilities.
    */
-  abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
+  abstract deprecated class SanitizerGuard extends DataFlow::BarrierGuard { }
 
   /**
-   * A source of exception info, considered as a flow source.
+   * A call to `tarfile.open`, considered as a flow source.
    */
   class TarfileOpen extends Source {
     TarfileOpen() {
       this = API::moduleImport("tarfile").getMember("open").getACall() and
       // If argument refers to a string object, then it's a hardcoded path and
       // this tarfile is safe.
       not this.(DataFlow::CallCfgNode).getArg(0).getALocalSource().asExpr() instanceof StrConst and
-      /* Ignore opens within the tarfile module itself */
+      // Ignore opens within the tarfile module itself
       not this.getLocation().getFile().getBaseName() = "tarfile.py"
     }
   }
@@ -110,33 +112,38 @@ module TarSlip {
   }
 
   /**
-   * A sanitizer guard heuristic.
+   * Holds if `g` clears taint for `tarInfo`.
    *
    * The test `if <check_path>(info.name)` should clear taint for `info`,
    * where `<check_path>` is any function matching `"%path"`.
    * `info` is assumed to be a `TarInfo` instance.
    */
-  class TarFileInfoSanitizer extends SanitizerGuard {
-    ControlFlowNode tarInfo;
+  predicate tarFileInfoSanitizer(DataFlow::GuardNode g, ControlFlowNode tarInfo, boolean branch) {
+    exists(CallNode call, AttrNode attr |
+      g = call and
+      // We must test the name of the tar info object.
+      attr = call.getAnArg() and
+      attr.getName() = "name" and
+      attr.getObject() = tarInfo
+    |
+      // Assume that any test with "path" in it is a sanitizer
+      call.getAChild*().(AttrNode).getName().matches("%path")
+      or
+      call.getAChild*().(NameNode).getId().matches("%path")
+    ) and
+    branch in [true, false]
+  }
 
+  /**
+   * A sanitizer guard heuristic.
+   *
+   * The test `if <check_path>(info.name)` should clear taint for `info`,
+   * where `<check_path>` is any function matching `"%path"`.
+   * `info` is assumed to be a `TarInfo` instance.
+   */
+  class TarFileInfoSanitizer extends Sanitizer {
     TarFileInfoSanitizer() {
-      exists(CallNode call, AttrNode attr |
-        this = call and
-        // We must test the name of the tar info object.
-        attr = call.getAnArg() and
-        attr.getName() = "name" and
-        attr.getObject() = tarInfo
-      |
-        // Assume that any test with "path" in it is a sanitizer
-        call.getAChild*().(AttrNode).getName().matches("%path")
-        or
-        call.getAChild*().(NameNode).getId().matches("%path")
-      )
-    }
-
-    override predicate checks(ControlFlowNode checked, boolean branch) {
-      checked = tarInfo and
-      branch in [true, false]
+      this = DataFlow::BarrierGuard<tarFileInfoSanitizer/3>::getABarrierNode()
     }
   }
 }

python/ql/lib/semmle/python/security/dataflow/TarSlip.qll renamed to python/ql/lib/semmle/python/security/dataflow/TarSlipQuery.qll

@@ -23,7 +23,7 @@ class Configuration extends TaintTracking::Configuration {
 
   override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
 
-  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+  deprecated override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
     guard instanceof SanitizerGuard
   }
 }

python/ql/src/Security/CWE-022/TarSlip.ql

@@ -13,7 +13,7 @@
  */
 
 import python
-import semmle.python.security.dataflow.TarSlip
+import semmle.python.security.dataflow.TarSlipQuery
 import DataFlow::PathGraph
 
 from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink