Merge pull request #1157 from markshannon/python-taint-tracking-early-exit

Python taint-tracking improvements

Author: taus-semmle

python/ql/src/semmle/python/security/TaintTracking.qll

@@ -1237,7 +1237,7 @@ library module TaintFlowImplementation {
             )
             |
             not Filters::isinstance(test.getTest(), _, var.getSourceVariable().getAUse()) and
-            not test.getTest() = var.getSourceVariable().getAUse()
+            not boolean_filter(test.getTest(), var.getSourceVariable().getAUse())
             or
             exists(ControlFlowNode c, ClassObject cls |
                 Filters::isinstance(test.getTest(), c, var.getSourceVariable().getAUse())
@@ -1248,10 +1248,42 @@ library module TaintFlowImplementation {
                 test.getSense() = false and not kind.getClass().getAnImproperSuperType() = cls
             )
             or
-            test.getTest() = var.getSourceVariable().getAUse() and kind.booleanValue() = test.getSense()
+            test.getSense() = test_evaluates(test.getTest(), var.getSourceVariable().getAUse(), kind)
         )
     }
 
+    /** Gets the operand of a unary `not` expression. */
+    private ControlFlowNode not_operand(ControlFlowNode expr) {
+        expr.(UnaryExprNode).getNode().getOp() instanceof Not and
+        result = expr.(UnaryExprNode).getOperand()
+    }
+
+    /** Holds if `test` is the test in a branch and `use` is that test
+     * with all the `not` prefixes removed.
+     */
+    private predicate boolean_filter(ControlFlowNode test, ControlFlowNode use) {
+        any(PyEdgeRefinement ref).getTest() = test and
+        (
+            use = test
+            or
+            exists(ControlFlowNode notuse |
+                boolean_filter(test, notuse) and
+                use = not_operand(notuse)
+            )
+        )
+    }
+
+    /** 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_filter(_, use) and
+        kind.taints(use) and
+        test = use and result = kind.booleanValue()
+        or
+        result = test_evaluates(not_operand(test), use, kind).booleanNot()
+    }
+
     pragma [noinline]
     predicate tainted_argument(ArgumentRefinement def, CallContext context, TaintedNode origin) {
         tainted_var(def.getInput(), context, origin)

python/ql/test/library-tests/taint/general/TaintLib.qll

@@ -334,3 +334,35 @@ class OsCommand extends TaintSink {
     }
 
 }
+
+
+class Falsey extends TaintKind {
+
+    Falsey() { this = "falsey" }
+
+    override boolean booleanValue() {
+        result = false
+    }
+
+}
+
+class FalseySource  extends TaintSource {
+
+    FalseySource() {
+         this.(NameNode).getId() = "FALSEY"
+    }
+
+    override predicate isSourceOf(TaintKind kind) {
+        kind instanceof Falsey
+    }
+
+    override string toString() {
+        result = "falsey.source"
+    }
+
+}
+
+
+
+
+

python/ql/test/library-tests/taint/general/TestDefn.expected

@@ -0,0 +1,9 @@
+import python
+import semmle.python.security.TaintTest
+import TaintLib
+
+
+from EssaDefinition defn, TaintedNode n
+where TaintFlowTest::tainted_def(defn, _, n)
+select 
+    defn.getLocation().toString(), defn.getRepresentation(), n.getLocation().toString(), n.getTrackedValue(), n.getNode().getNode().toString()

python/ql/test/library-tests/taint/general/TestDefn.ql

@@ -94,6 +94,8 @@
 | Taint explicit.carrier | carrier.py:33 | TAINT_CARRIER_SOURCE |  |
 | Taint explicit.carrier | carrier.py:34 | Attribute |  |
 | Taint explicit.carrier | carrier.py:35 | x |  |
+| Taint falsey | test.py:189 | FALSEY |  |
+| Taint falsey | test.py:190 | t |  |
 | Taint paper | rockpaperscissors.py:6 | arg | rockpaperscissors.py:32 |
 | Taint paper | rockpaperscissors.py:9 | arg | rockpaperscissors.py:26 |
 | Taint paper | rockpaperscissors.py:25 | Attribute() |  |

python/ql/test/library-tests/taint/general/TestNode.expected

@@ -41,3 +41,4 @@
 | test.py:168 | SOURCE | simple.test |
 | test.py:169 | SOURCE | simple.test |
 | test.py:178 | SOURCE | simple.test |
+| test.py:189 | FALSEY | falsey |

python/ql/test/library-tests/taint/general/TestSource.expected

@@ -82,6 +82,7 @@
 | Taint explicit.carrier | carrier.py:33 | TAINT_CARRIER_SOURCE |  |  -->  | Taint explicit.carrier | carrier.py:4 | arg | carrier.py:33 |
 | Taint explicit.carrier | carrier.py:34 | Attribute |  |  -->  | Taint explicit.carrier | carrier.py:35 | x |  |
 | Taint explicit.carrier | carrier.py:35 | x |  |  -->  | Taint simple.test | carrier.py:35 | Attribute() |  |
+| Taint falsey | test.py:189 | FALSEY |  |  -->  | Taint falsey | test.py:190 | t |  |
 | Taint paper | rockpaperscissors.py:25 | Attribute() |  |  -->  | Taint paper | rockpaperscissors.py:26 | y |  |
 | Taint paper | rockpaperscissors.py:26 | y |  |  -->  | Taint paper | rockpaperscissors.py:9 | arg | rockpaperscissors.py:26 |
 | Taint paper | rockpaperscissors.py:30 | Attribute() |  |  -->  | Taint paper | rockpaperscissors.py:32 | y |  |

python/ql/test/library-tests/taint/general/TestStep.expected

@@ -182,3 +182,5 @@
 | test.py:180 | t_2 | test.py:178 | Taint simple.test | SOURCE |
 | test.py:183 | t_3 | test.py:178 | Taint simple.test | SOURCE |
 | test.py:186 | t_4 | test.py:178 | Taint simple.test | SOURCE |
+| test.py:189 | t_0 | test.py:189 | Taint falsey | FALSEY |
+| test.py:191 | t_1 | test.py:189 | Taint falsey | FALSEY |

python/ql/test/library-tests/taint/general/TestVar.expected

@@ -184,3 +184,9 @@ def test_truth():
         SINK(t)
     else:
         SINK(t)
+
+def test_early_exit():
+    t = FALSEY
+    if not t:
+        return
+    t

python/ql/test/library-tests/taint/general/test.py

@@ -10,6 +10,7 @@ edges
 | functions_test.py:298:21:298:21 | empty mutable value | functions_test.py:293:21:293:21 | empty mutable value |
 | functions_test.py:300:26:300:26 | empty mutable value | functions_test.py:301:8:301:8 | empty mutable value |
 | 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 |
 parents
 | functions_test.py:290:25:290:25 | empty mutable value | functions_test.py:297:25:297:25 | empty mutable value |
 | functions_test.py:291:5:291:5 | empty mutable value | functions_test.py:297:25:297:25 | empty mutable value |