diff --git a/python/ql/src/Functions/ModificationOfParameterWithDefault.ql b/python/ql/src/Functions/ModificationOfParameterWithDefault.ql index ab94a9735763..04cd3e798328 100644 --- a/python/ql/src/Functions/ModificationOfParameterWithDefault.ql +++ b/python/ql/src/Functions/ModificationOfParameterWithDefault.ql @@ -15,56 +15,50 @@ import python import semmle.python.security.Paths predicate safe_method(string name) { - name = "count" or name = "index" or name = "copy" or name = "get" or name = "has_key" or - name = "items" or name = "keys" or name = "values" or name = "iteritems" or name = "iterkeys" or name = "itervalues" + name = "count" or + name = "index" or + name = "copy" or + name = "get" or + name = "has_key" or + name = "items" or + name = "keys" or + name = "values" or + name = "iteritems" or + name = "iterkeys" or + name = "itervalues" } /** 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(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(List l | p.getDefault() = l | exists(l.getAnElt()) and result = true or not exists(l.getAnElt()) and result = false ) } - class NonEmptyMutableValue extends TaintKind { - NonEmptyMutableValue() { - this = "non-empty mutable value" - } + NonEmptyMutableValue() { this = "non-empty mutable value" } } class EmptyMutableValue extends TaintKind { - EmptyMutableValue() { - this = "empty mutable value" - } - - override boolean booleanValue() { - result = false - } + EmptyMutableValue() { this = "empty mutable value" } + override boolean booleanValue() { result = false } } class MutableDefaultValue extends TaintSource { - boolean nonEmpty; - MutableDefaultValue() { - nonEmpty = mutableDefaultValue(this.(NameNode).getNode()) - } + MutableDefaultValue() { nonEmpty = mutableDefaultValue(this.(NameNode).getNode()) } - override string toString() { - result = "mutable default value" - } + override string toString() { result = "mutable default value" } override predicate isSourceOf(TaintKind kind) { nonEmpty = false and kind instanceof EmptyMutableValue @@ -73,14 +67,19 @@ class MutableDefaultValue extends TaintSource { } } +private ClassValue mutable_class() { + result = Value::named("list") or + result = Value::named("dict") +} + class Mutation extends TaintSink { Mutation() { exists(AugAssign a | a.getTarget().getAFlowNode() = this) or - exists(Call c, Attribute a | - c.getFunc() = a | + exists(Call c, Attribute a | c.getFunc() = a | a.getObject().getAFlowNode() = this and - not safe_method(a.getName()) + not safe_method(a.getName()) and + this.(ControlFlowNode).pointsTo().getClass() = mutable_class() ) } @@ -93,4 +92,5 @@ class Mutation extends TaintSink { from TaintedPathSource src, TaintedPathSink sink where src.flowsTo(sink) -select sink.getSink(), src, sink, "$@ flows to here and is mutated.", src.getSource(), "Default value" +select sink.getSink(), src, sink, "$@ flows to here and is mutated.", src.getSource(), + "Default value" diff --git a/python/ql/test/query-tests/Functions/general/ModificationOfParameterWithDefault.expected b/python/ql/test/query-tests/Functions/general/ModificationOfParameterWithDefault.expected index 9b7af6df8b72..08586d02c100 100644 --- a/python/ql/test/query-tests/Functions/general/ModificationOfParameterWithDefault.expected +++ b/python/ql/test/query-tests/Functions/general/ModificationOfParameterWithDefault.expected @@ -7,8 +7,18 @@ edges | functions_test.py:157:27:157:27 | empty mutable value | functions_test.py:159:21:159:21 | empty mutable value | | functions_test.py:158:25:158:25 | empty mutable value | functions_test.py:151:25:151:25 | empty mutable value | | functions_test.py:159:21:159:21 | empty mutable value | functions_test.py:154:21:154:21 | empty mutable value | +| functions_test.py:175:28:175:28 | non-empty mutable value | functions_test.py:179:9:179:9 | non-empty mutable value | +| functions_test.py:175:28:175:28 | non-empty mutable value | functions_test.py:181:9:181:9 | non-empty mutable value | +| functions_test.py:188:18:188:18 | non-empty mutable value | functions_test.py:189:28:189:28 | non-empty mutable value | +| functions_test.py:189:28:189:28 | non-empty mutable value | functions_test.py:175:28:175:28 | non-empty mutable value | +| functions_test.py:191:18:191:18 | non-empty mutable value | functions_test.py:192:28:192:28 | non-empty mutable value | +| functions_test.py:192:28:192:28 | non-empty mutable value | functions_test.py:175:28:175:28 | non-empty mutable value | #select | 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:134:5:134:5 | x | functions_test.py:133:15:133:15 | empty mutable value | functions_test.py:134:5:134:5 | empty mutable value | $@ flows to here and is mutated. | functions_test.py:133:15:133:15 | x | Default value | | functions_test.py:152:5:152:5 | x | functions_test.py:157:27:157:27 | empty mutable value | functions_test.py:152:5:152:5 | empty mutable value | $@ flows to here and is mutated. | functions_test.py:157:27:157:27 | y | Default value | | functions_test.py:155:5:155:5 | x | functions_test.py:157:27:157:27 | empty mutable value | functions_test.py:155:5:155:5 | empty mutable value | $@ flows to here and is mutated. | functions_test.py:157:27:157:27 | y | Default value | +| functions_test.py:179:9:179:9 | x | functions_test.py:188:18:188:18 | non-empty mutable value | functions_test.py:179:9:179:9 | non-empty mutable value | $@ flows to here and is mutated. | functions_test.py:188:18:188:18 | x | Default value | +| functions_test.py:179:9:179:9 | x | functions_test.py:191:18:191:18 | non-empty mutable value | functions_test.py:179:9:179:9 | non-empty mutable value | $@ flows to here and is mutated. | functions_test.py:191:18:191:18 | x | Default value | +| functions_test.py:181:9:181:9 | x | functions_test.py:188:18:188:18 | non-empty mutable value | functions_test.py:181:9:181:9 | non-empty mutable value | $@ flows to here and is mutated. | functions_test.py:188:18:188:18 | x | Default value | +| functions_test.py:181:9:181:9 | x | functions_test.py:191:18:191:18 | non-empty mutable value | functions_test.py:181:9:181:9 | non-empty mutable value | $@ flows to here and is mutated. | functions_test.py:191:18:191:18 | x | 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 271539792e72..367a5e54e660 100644 --- a/python/ql/test/query-tests/Functions/general/functions_test.py +++ b/python/ql/test/query-tests/Functions/general/functions_test.py @@ -168,3 +168,28 @@ def issue1143(expr, param=[]): return result for i in param: param.remove(i) # Mutation here + + +# Type guarding of modification of parameter with default: + +def do_stuff_based_on_type(x): + if isinstance(x, str): + x = x.split() + elif isinstance(x, dict): + x.setdefault('foo', 'bar') + elif isinstance(x, list): + x.append(5) + elif isinstance(x, tuple): + x = x.unknown_method() + +def str_default(x="hello world"): + do_stuff_based_on_type(x) + +def dict_default(x={'baz':'quux'}): + do_stuff_based_on_type(x) + +def list_default(x=[1,2,3,4]): + do_stuff_based_on_type(x) + +def tuple_default(x=(1,2)): + do_stuff_based_on_type(x)