Python: Fix up pi-node handling in taint-tracking.

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

@@ -187,6 +187,18 @@ class TarSlipConfiguration extends TaintTracking::Configuration {
         sanitizer instanceof ExcludeTarFilePy
     }
 
+    override predicate isBarrier(DataFlow::Node node) {
+        // Avoid flow into the tarfile module
+        exists(ParameterDefinition def |
+            node.asVariable().getDefinition() = def
+            or
+            node.asCfgNode() = def.getDefiningNode()
+            |
+            def.getScope() = Value::named("tarfile.open").(CallableValue).getScope()
+            or
+            def.isSelf() and def.getScope().getEnclosingModule().getName() = "tarfile"
+        )
+    }
 }
 
 

python/ql/src/semmle/python/dataflow/Implementation.qll

@@ -288,17 +288,36 @@ class TaintTrackingImplementation extends string {
     /** Gets the boolean value that `test` evaluates to when `use` is tainted with `kind`
      * and `test` and `use` are part of a test in a branch.
      */
-    private boolean test_evaluates(ControlFlowNode test, ControlFlowNode use, TaintKind kind) {
+    boolean testEvaluates(ControlFlowNode test, ControlFlowNode use, TaintKind kind) {
         boolean_filter(_, use) and
         kind.taints(use) and
         test = use and result = kind.booleanValue()
         or
-        result = test_evaluates(not_operand(test), use, kind).booleanNot()
+        result = testEvaluates(not_operand(test), use, kind).booleanNot()
         or
         exists(ControlFlowNode const |
             Filters::equality_test(test, use, result.booleanNot(), const) and
             const.getNode() instanceof ImmutableLiteral
         )
+        or
+        exists(ControlFlowNode c, ClassValue cls |
+            Filters::isinstance(test, c, use) and
+            c.pointsTo(cls)
+            |
+            kind.getType().getASuperType() = cls and result = true
+            or
+            not kind.getType().getASuperType() = cls and result = false
+        )
+    }
+
+    predicate testEvaluatesMaybe(ControlFlowNode test, ControlFlowNode use) {
+        any(PyEdgeRefinement ref).getTest().getAChild*() = test and
+        test.getAChild*() = use and
+        not test.(UnaryExprNode).getNode().getOp() instanceof Not and
+        not Filters::equality_test(test, use, _, _) and
+        not Filters::isinstance(test, _, use)
+        or
+        testEvaluatesMaybe(not_operand(test), use)
     }
 
     /** Gets the operand of a unary `not` expression. */
@@ -656,16 +675,9 @@ class TaintTrackingImplementation extends string {
             srcnode.asVariable() = defn.getInput() and
             not this.(TaintTracking::Configuration).isBarrierTest(defn.getTest(), defn.getSense())
             |
-            exists(ControlFlowNode c, ClassValue cls |
-                Filters::isinstance(defn.getTest(), c, defn.getInput().getSourceVariable().getAUse()) and
-                c.pointsTo(cls)
-                |
-                defn.getSense() = true and kind.getType().getASuperType() = cls
-                or
-                defn.getSense() = false and not kind.getType().getASuperType() = cls
-            )
+            defn.getSense() = testEvaluates(defn.getTest(), defn.getInput().getSourceVariable().getAUse(), kind)
             or
-            defn.getSense() = test_evaluates(defn.getTest(), defn.getInput().getSourceVariable().getAUse(), kind)
+            testEvaluatesMaybe(defn.getTest(), defn.getInput().getSourceVariable().getAUse())
         )
     }
 

python/ql/test/query-tests/Functions/general/ModificationOfParameterWithDefault.expected

@@ -12,7 +12,7 @@ edges
 | functions_test.py:300:26:300:26 | empty mutable value | functions_test.py:303:12:303:12 | empty mutable value |
 | functions_test.py:305:21:305:25 | empty mutable value | functions_test.py:306:12:306:16 | empty mutable value |
 #select
-| functions_test.py:40:5:40:5 | Taint sink | functions_test.py:39:9:39:9 | empty mutable value | functions_test.py:40:5:40:5 | empty mutable value | $@ flows to here and is mutated. | functions_test.py:39:9:39:9 | mutable default value | Default value |
-| functions_test.py:239:5:239:5 | Taint sink | functions_test.py:238:15:238:15 | empty mutable value | functions_test.py:239:5:239:5 | empty mutable value | $@ flows to here and is mutated. | functions_test.py:238:15:238:15 | mutable default value | Default value |
-| functions_test.py:291:5:291:5 | Taint sink | functions_test.py:296:27:296:27 | empty mutable value | functions_test.py:291:5:291:5 | empty mutable value | $@ flows to here and is mutated. | functions_test.py:296:27:296:27 | mutable default value | Default value |
-| functions_test.py:294:5:294:5 | Taint sink | functions_test.py:296:27:296:27 | empty mutable value | functions_test.py:294:5:294:5 | empty mutable value | $@ flows to here and is mutated. | functions_test.py:296:27:296:27 | mutable default value | Default value |
+| functions_test.py:40:5:40:5 | x | functions_test.py:39:9:39:9 | empty mutable value | functions_test.py:40:5:40:5 | empty mutable value | $@ flows to here and is mutated. | functions_test.py:39:9:39:9 | x | Default value |
+| functions_test.py:239:5:239:5 | x | functions_test.py:238:15:238:15 | empty mutable value | functions_test.py:239:5:239:5 | empty mutable value | $@ flows to here and is mutated. | functions_test.py:238:15:238:15 | x | Default value |
+| functions_test.py:291:5:291:5 | x | functions_test.py:296:27:296:27 | empty mutable value | functions_test.py:291:5:291:5 | empty mutable value | $@ flows to here and is mutated. | functions_test.py:296:27:296:27 | y | Default value |
+| functions_test.py:294:5:294:5 | x | functions_test.py:296:27:296:27 | empty mutable value | functions_test.py:294:5:294:5 | empty mutable value | $@ flows to here and is mutated. | functions_test.py:296:27:296:27 | y | Default value |

python/ql/test/query-tests/Security/CWE-327/TestNode.expected

@@ -1,3 +1,4 @@
+WARNING: Predicate getNode has been deprecated and may be removed in future (TestNode.ql:9,25-32)
 | Taint Crypto.Cipher.ARC4 | test_pycrypto.py:5:14:5:27 | test_pycrypto.py:5 | test_pycrypto.py:5:14:5:27 | Attribute() |  |
 | Taint Crypto.Cipher.ARC4 | test_pycrypto.py:6:12:6:17 | test_pycrypto.py:6 | test_pycrypto.py:6:12:6:17 | cipher |  |
 | Taint cryptography.Cipher.RC4 | test_cryptography.py:5:14:5:47 | test_cryptography.py:5 | test_cryptography.py:5:14:5:47 | Cipher() |  |
@@ -8,7 +9,6 @@
 | Taint sensitive.data | test_cryptography.py:4:17:4:28 | test_cryptography.py:4 | test_cryptography.py:4:17:4:28 | get_password |  |
 | Taint sensitive.data | test_cryptography.py:4:17:4:30 | test_cryptography.py:4 | test_cryptography.py:4:17:4:30 | get_password() |  |
 | Taint sensitive.data | test_cryptography.py:7:29:7:37 | test_cryptography.py:7 | test_cryptography.py:7:29:7:37 | dangerous |  |
-| Taint sensitive.data | test_cryptography.py:7:42:7:50 | test_cryptography.py:7 | test_cryptography.py:7:42:7:50 | encryptor |  |
 | Taint sensitive.data | test_pycrypto.py:4:17:4:28 | test_pycrypto.py:4 | test_pycrypto.py:4:17:4:28 | get_password |  |
 | Taint sensitive.data | test_pycrypto.py:4:17:4:30 | test_pycrypto.py:4 | test_pycrypto.py:4:17:4:30 | get_password() |  |
 | Taint sensitive.data | test_pycrypto.py:6:27:6:35 | test_pycrypto.py:6 | test_pycrypto.py:6:27:6:35 | dangerous |  |

python/ql/test/query-tests/Security/CWE-327/TestNode.ql

@@ -7,4 +7,4 @@ import semmle.python.security.Crypto
 
 from TaintedNode n, AstNode src
 where src = n.getAstNode() and src.getLocation().getFile().getName().matches("%test%")
-select n.getTrackedValue(), n.getLocation(), src, n.getContext()
+select "Taint " + n.getTaintKind(), n.getLocation(), src, n.getContext()