diff --git a/python/ql/src/Functions/ModificationOfParameterWithDefault.ql b/python/ql/src/Functions/ModificationOfParameterWithDefault.ql index 206174e02b6..ab94a973576 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 -} - -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() - ) +/** 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 ) } -class MutableValue extends TaintKind { - MutableValue() { - this = "mutable value" + +class NonEmptyMutableValue extends TaintKind { + NonEmptyMutableValue() { + this = "non-empty 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/test/query-tests/Functions/general/ModificationOfParameterWithDefault.expected b/python/ql/test/query-tests/Functions/general/ModificationOfParameterWithDefault.expected index fa8feaf186b..a65d2ca8dc1 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 280af2bc227..91613675aec 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 +