diff --git a/python/ql/src/Functions/ModificationOfParameterWithDefault.ql b/python/ql/src/Functions/ModificationOfParameterWithDefault.ql index 206174e02b6a..ab94a9735763 100644 --- a/python/ql/src/Functions/ModificationOfParameterWithDefault.ql +++ b/python/ql/src/Functions/ModificationOfParameterWithDefault.ql @@ -19,32 +19,47 @@ predicate safe_method(string name) { name = "items" or name = "keys" or name = "values" or name = "iteritems" or name = "iterkeys" or name = "itervalues" } -predicate maybe_parameter(SsaVariable var, Function f, Parameter p) { - p = var.getAnUltimateDefinition().getDefinition().getNode() and - f.getAnArg() = p +/** Gets the truthiness (non emptyness) of the default of `p` if that value is mutable */ +private boolean mutableDefaultValue(Parameter p) { + exists(Dict d | + p.getDefault() = d | + exists(d.getAKey()) and result = true + or + not exists(d.getAKey()) and result = false + ) + or + exists(List l | + p.getDefault() = l | + exists(l.getAnElt()) and result = true + or + not exists(l.getAnElt()) and result = false + ) } -predicate has_mutable_default(Parameter p) { - exists(SsaVariable v, FunctionExpr f | maybe_parameter(v, f.getInnerScope(), p) and - exists(int i, int def_cnt, int arg_cnt | - def_cnt = count(f.getArgs().getADefault()) and - arg_cnt = count(f.getInnerScope().getAnArg()) and - i in [1 .. arg_cnt] and - (f.getArgs().getDefault(def_cnt - i) instanceof Dict or f.getArgs().getDefault(def_cnt - i) instanceof List) and - f.getInnerScope().getArgName(arg_cnt - i) = v.getId() - ) - ) + +class NonEmptyMutableValue extends TaintKind { + NonEmptyMutableValue() { + this = "non-empty mutable value" + } } -class MutableValue extends TaintKind { - MutableValue() { - this = "mutable value" +class EmptyMutableValue extends TaintKind { + EmptyMutableValue() { + this = "empty mutable value" } + + override boolean booleanValue() { + result = false + } + } class MutableDefaultValue extends TaintSource { + + boolean nonEmpty; + MutableDefaultValue() { - has_mutable_default(this.(NameNode).getNode()) + nonEmpty = mutableDefaultValue(this.(NameNode).getNode()) } override string toString() { @@ -52,7 +67,9 @@ class MutableDefaultValue extends TaintSource { } override predicate isSourceOf(TaintKind kind) { - kind instanceof MutableValue + nonEmpty = false and kind instanceof EmptyMutableValue + or + nonEmpty = true and kind instanceof NonEmptyMutableValue } } @@ -68,7 +85,9 @@ class Mutation extends TaintSink { } override predicate sinks(TaintKind kind) { - kind instanceof MutableValue + kind instanceof EmptyMutableValue + or + kind instanceof NonEmptyMutableValue } } diff --git a/python/ql/src/semmle/python/security/TaintTracking.qll b/python/ql/src/semmle/python/security/TaintTracking.qll index bd0037b52caf..16cd1952a90d 100755 --- a/python/ql/src/semmle/python/security/TaintTracking.qll +++ b/python/ql/src/semmle/python/security/TaintTracking.qll @@ -148,6 +148,16 @@ abstract class TaintKind extends string { none() } + /** Gets the boolean values (may be one, neither, or both) that + * may result from the Python expression `bool(this)` + */ + boolean booleanValue() { + /* Default to true as the vast majority of taint is strings and + * the empty string is almost always benign. + */ + result = true + } + string repr() { result = this } } @@ -1190,7 +1200,8 @@ library module TaintFlowImplementation { sanitizer.sanitizingEdge(kind, test) ) | - not Filters::isinstance(test.getTest(), _, var.getSourceVariable().getAUse()) + not Filters::isinstance(test.getTest(), _, var.getSourceVariable().getAUse()) and + not test.getTest() = var.getSourceVariable().getAUse() or exists(ControlFlowNode c, ClassObject cls | Filters::isinstance(test.getTest(), c, var.getSourceVariable().getAUse()) @@ -1200,6 +1211,8 @@ library module TaintFlowImplementation { or test.getSense() = false and not kind.getClass().getAnImproperSuperType() = cls ) + or + test.getTest() = var.getSourceVariable().getAUse() and kind.booleanValue() = test.getSense() ) } diff --git a/python/ql/test/library-tests/taint/general/TestNode.expected b/python/ql/test/library-tests/taint/general/TestNode.expected index c7ae50289a07..b1f290b4acba 100644 --- a/python/ql/test/library-tests/taint/general/TestNode.expected +++ b/python/ql/test/library-tests/taint/general/TestNode.expected @@ -215,6 +215,11 @@ | Taint simple.test | test.py:169 | SOURCE | | | Taint simple.test | test.py:172 | Subscript | | | Taint simple.test | test.py:173 | Subscript | | +| Taint simple.test | test.py:178 | SOURCE | | +| Taint simple.test | test.py:179 | t | | +| Taint simple.test | test.py:180 | t | | +| Taint simple.test | test.py:183 | t | | +| Taint simple.test | test.py:186 | t | | | Taint {simple.test} | test.py:169 | Dict | | | Taint {simple.test} | test.py:171 | d | | | Taint {simple.test} | test.py:173 | y | | diff --git a/python/ql/test/library-tests/taint/general/TestSink.expected b/python/ql/test/library-tests/taint/general/TestSink.expected index dd169d78f033..c8f7f22a1fa4 100644 --- a/python/ql/test/library-tests/taint/general/TestSink.expected +++ b/python/ql/test/library-tests/taint/general/TestSink.expected @@ -32,3 +32,5 @@ | simple.test | test.py:159 | 160 | t | simple.test | | simple.test | test.py:168 | 172 | Subscript | simple.test | | simple.test | test.py:169 | 173 | Subscript | simple.test | +| simple.test | test.py:178 | 180 | t | simple.test | +| simple.test | test.py:178 | 186 | t | simple.test | diff --git a/python/ql/test/library-tests/taint/general/TestSource.expected b/python/ql/test/library-tests/taint/general/TestSource.expected index ea9012b7735b..d341758b2eae 100644 --- a/python/ql/test/library-tests/taint/general/TestSource.expected +++ b/python/ql/test/library-tests/taint/general/TestSource.expected @@ -40,3 +40,4 @@ | test.py:163 | SOURCE | simple.test | | test.py:168 | SOURCE | simple.test | | test.py:169 | SOURCE | simple.test | +| test.py:178 | SOURCE | simple.test | diff --git a/python/ql/test/library-tests/taint/general/TestStep.expected b/python/ql/test/library-tests/taint/general/TestStep.expected index a4c72fd6b0ce..dec3e5b03b24 100644 --- a/python/ql/test/library-tests/taint/general/TestStep.expected +++ b/python/ql/test/library-tests/taint/general/TestStep.expected @@ -173,6 +173,10 @@ | Taint simple.test | test.py:163 | SOURCE | | --> | Taint simple.test | test.py:164 | s | | | Taint simple.test | test.py:168 | SOURCE | | --> | Taint [simple.test] | test.py:168 | List | | | Taint simple.test | test.py:169 | SOURCE | | --> | Taint {simple.test} | test.py:169 | Dict | | +| Taint simple.test | test.py:178 | SOURCE | | --> | Taint simple.test | test.py:179 | t | | +| Taint simple.test | test.py:178 | SOURCE | | --> | Taint simple.test | test.py:180 | t | | +| Taint simple.test | test.py:178 | SOURCE | | --> | Taint simple.test | test.py:183 | t | | +| Taint simple.test | test.py:178 | SOURCE | | --> | Taint simple.test | test.py:186 | t | | | Taint {simple.test} | test.py:169 | Dict | | --> | Taint {simple.test} | test.py:171 | d | | | Taint {simple.test} | test.py:169 | Dict | | --> | Taint {simple.test} | test.py:175 | d | | | Taint {simple.test} | test.py:171 | d | | --> | Taint {simple.test} | test.py:173 | y | | diff --git a/python/ql/test/library-tests/taint/general/TestVar.expected b/python/ql/test/library-tests/taint/general/TestVar.expected index 5aad11fb5476..9939820a0a4d 100644 --- a/python/ql/test/library-tests/taint/general/TestVar.expected +++ b/python/ql/test/library-tests/taint/general/TestVar.expected @@ -177,3 +177,8 @@ | test.py:174 | l_2 | test.py:168 | Taint [simple.test] | List | | test.py:175 | d2_0 | test.py:175 | Taint {simple.test} | dict() | | test.py:175 | d_2 | test.py:169 | Taint {simple.test} | Dict | +| test.py:178 | t_0 | test.py:178 | Taint simple.test | SOURCE | +| test.py:180 | t_1 | test.py:178 | Taint simple.test | SOURCE | +| 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 | diff --git a/python/ql/test/library-tests/taint/general/test.py b/python/ql/test/library-tests/taint/general/test.py index 5a6fc1e2900d..6c752b2c1d71 100644 --- a/python/ql/test/library-tests/taint/general/test.py +++ b/python/ql/test/library-tests/taint/general/test.py @@ -173,3 +173,14 @@ def test_update_extend(x, y): SINK(y["key"]) l2 = list(l) d2 = dict(d) + +def test_truth(): + t = SOURCE + if t: + SINK(t) + else: + SINK(t) + if not t: + SINK(t) + else: + SINK(t) diff --git a/python/ql/test/query-tests/Functions/general/ModificationOfParameterWithDefault.expected b/python/ql/test/query-tests/Functions/general/ModificationOfParameterWithDefault.expected index fa8feaf186ba..a65d2ca8dc15 100644 --- a/python/ql/test/query-tests/Functions/general/ModificationOfParameterWithDefault.expected +++ b/python/ql/test/query-tests/Functions/general/ModificationOfParameterWithDefault.expected @@ -1,20 +1,22 @@ edges -| functions_test.py:36:9:36:9 | mutable value | functions_test.py:37:16:37:16 | mutable value | -| functions_test.py:39:9:39:9 | mutable value | functions_test.py:40:5:40:5 | mutable value | -| functions_test.py:238:15:238:15 | mutable value | functions_test.py:239:5:239:5 | mutable value | -| functions_test.py:290:25:290:25 | mutable value | functions_test.py:291:5:291:5 | mutable value | -| functions_test.py:293:21:293:21 | mutable value | functions_test.py:294:5:294:5 | mutable value | -| functions_test.py:296:27:296:27 | mutable value | functions_test.py:297:25:297:25 | mutable value | -| functions_test.py:296:27:296:27 | mutable value | functions_test.py:298:21:298:21 | mutable value | -| functions_test.py:297:25:297:25 | mutable value | functions_test.py:290:25:290:25 | mutable value | -| functions_test.py:298:21:298:21 | mutable value | functions_test.py:293:21:293:21 | mutable value | +| functions_test.py:36:9:36:9 | empty mutable value | functions_test.py:37:16:37:16 | empty mutable value | +| functions_test.py:39:9:39:9 | empty mutable value | functions_test.py:40:5:40:5 | empty mutable value | +| functions_test.py:238:15:238:15 | empty mutable value | functions_test.py:239:5:239:5 | empty mutable value | +| functions_test.py:290:25:290:25 | empty mutable value | functions_test.py:291:5:291:5 | empty mutable value | +| functions_test.py:293:21:293:21 | empty mutable value | functions_test.py:294:5:294:5 | empty mutable value | +| functions_test.py:296:27:296:27 | empty mutable value | functions_test.py:297:25:297:25 | empty mutable value | +| functions_test.py:296:27:296:27 | empty mutable value | functions_test.py:298:21:298:21 | empty mutable value | +| functions_test.py:297:25:297:25 | empty mutable value | functions_test.py:290:25:290:25 | empty mutable value | +| 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 | parents -| functions_test.py:290:25:290:25 | mutable value | functions_test.py:297:25:297:25 | mutable value | -| functions_test.py:291:5:291:5 | mutable value | functions_test.py:297:25:297:25 | mutable value | -| functions_test.py:293:21:293:21 | mutable value | functions_test.py:298:21:298:21 | mutable value | -| functions_test.py:294:5:294:5 | mutable value | functions_test.py:298:21:298:21 | mutable value | +| 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 | +| functions_test.py:293:21:293:21 | empty mutable value | functions_test.py:298:21:298:21 | empty mutable value | +| functions_test.py:294:5:294:5 | empty mutable value | functions_test.py:298:21:298:21 | empty mutable value | #select -| functions_test.py:40:5:40:5 | Taint sink | functions_test.py:39:9:39:9 | mutable value | functions_test.py:40:5:40:5 | 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 | mutable value | functions_test.py:239:5:239:5 | 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 | mutable value | functions_test.py:291:5:291:5 | 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 | mutable value | functions_test.py:294:5:294:5 | 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 | 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 | diff --git a/python/ql/test/query-tests/Functions/general/functions_test.py b/python/ql/test/query-tests/Functions/general/functions_test.py index 280af2bc2279..91613675aec1 100644 --- a/python/ql/test/query-tests/Functions/general/functions_test.py +++ b/python/ql/test/query-tests/Functions/general/functions_test.py @@ -296,3 +296,9 @@ def mutate_argument(x): def indirect_modification(y = []): aug_assign_argument(y) mutate_argument(y) + +def guarded_modification(z=[]): + if z: + z.append(0) + return z +