Python: Make "Modification of parameter with default" flow-sensitive.

python/ql/src/Functions/ModificationOfParameterWithDefault.ql

@@ -12,6 +12,7 @@
  */
 
 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
@@ -23,27 +24,6 @@ predicate maybe_parameter(SsaVariable var, Function f, Parameter p) {
     f.getAnArg() = p
 }
 
-Name use_of_parameter(Parameter p) {
-    exists(SsaVariable var |
-        p = var.getAnUltimateDefinition().getDefinition().getNode() and
-        var.getAUse().getNode() = result
-    )
-}
-
-predicate modifying_call(Call c, Parameter p) {
-    exists(Attribute a |
-        c.getFunc() = a |
-        a.getObject() = use_of_parameter(p) and
-        not safe_method(a.getName())
-    )
-}
-
-predicate is_modification(AstNode a, Parameter p) {
-    modifying_call(a, p)
-    or
-    a.(AugAssign).getTarget() = use_of_parameter(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 |
@@ -56,6 +36,42 @@ predicate has_mutable_default(Parameter p) {
     )
 }
 
-from AstNode a, Parameter p
-where has_mutable_default(p) and is_modification(a, p)
-select a, "Modification of parameter $@, which has mutable default value.", p, p.asName().getId()
+class MutableValue extends TaintKind {
+    MutableValue() {
+        this = "mutable value"
+    }
+}
+
+class MutableDefaultValue extends TaintSource {
+    MutableDefaultValue() {
+        has_mutable_default(this.(NameNode).getNode())
+    }
+
+    override string toString() {
+        result = "mutable default value"
+    }
+
+    override predicate isSourceOf(TaintKind kind) {
+        kind instanceof MutableValue
+    }
+}
+
+class Mutation extends TaintSink {
+    Mutation() {
+        exists(AugAssign a | a.getTarget().getAFlowNode() = this)
+        or
+        exists(Call c, Attribute a |
+            c.getFunc() = a |
+            a.getObject().getAFlowNode() = this and
+            not safe_method(a.getName())
+        )
+    }
+
+    override predicate sinks(TaintKind kind) {
+        kind instanceof MutableValue
+    }
+}
+
+from TaintedPathSource src, TaintedPathSink sink
+where src.flowsTo(sink)
+select sink.getSink(), src, sink, "$@ flows to here and is mutated.", src.getSource(), "Default value"

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

@@ -1,2 +1,20 @@
-| functions_test.py:40:5:40:17 | Attribute() | Modification of parameter $@, which has mutable default value. | functions_test.py:39:9:39:9 | Parameter | x |
-| functions_test.py:239:5:239:14 | AugAssign | Modification of parameter $@, which has mutable default value. | functions_test.py:238:15:238:15 | Parameter | x |
+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 |
+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 |
+#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 |

python/ql/test/query-tests/Functions/general/functions_test.py

@@ -286,3 +286,13 @@ class Z(zope.interface.Interface):
 
 Z().meth(0)
 
+# indirect modification of parameter with default
+def aug_assign_argument(x):
+    x += ['x']
+
+def mutate_argument(x):
+    x.append('x')
+
+def indirect_modification(y = []):
+    aug_assign_argument(y)
+    mutate_argument(y)