Skip to content

Commit e865a29

Browse files
yoffyoff
committed
Python: straight port of query
The old query uses `pointsTo` to limit the sinks to methods on lists and dictionaries. That constraint is omitted here which could hurt performance.
1 parent e3765ce commit e865a29

File tree

5 files changed

+171
-112
lines changed

5 files changed

+171
-112
lines changed

‎python/ql/src/Functions/ModificationOfParameterWithDefault.ql

Lines changed: 8 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -12,88 +12,12 @@
1212
*/
1313

1414
import python
15-
import semmle.python.security.Paths
16-
17-
predicate safe_method(string name) {
18-
name = "count" or
19-
name = "index" or
20-
name = "copy" or
21-
name = "get" or
22-
name = "has_key" or
23-
name = "items" or
24-
name = "keys" or
25-
name = "values" or
26-
name = "iteritems" or
27-
name = "iterkeys" or
28-
name = "itervalues" or
29-
name = "__contains__" or
30-
name = "__getitem__" or
31-
name = "__getattribute__"
32-
}
33-
34-
/** Gets the truthiness (non emptyness) of the default of `p` if that value is mutable */
35-
private boolean mutableDefaultValue(Parameter p) {
36-
exists(Dict d | p.getDefault() = d |
37-
exists(d.getAKey()) and result = true
38-
or
39-
not exists(d.getAKey()) and result = false
40-
)
41-
or
42-
exists(List l | p.getDefault() = l |
43-
exists(l.getAnElt()) and result = true
44-
or
45-
not exists(l.getAnElt()) and result = false
46-
)
47-
}
48-
49-
class NonEmptyMutableValue extends TaintKind {
50-
NonEmptyMutableValue() { this = "non-empty mutable value" }
51-
}
52-
53-
class EmptyMutableValue extends TaintKind {
54-
EmptyMutableValue() { this = "empty mutable value" }
55-
56-
override boolean booleanValue() { result = false }
57-
}
58-
59-
class MutableDefaultValue extends TaintSource {
60-
boolean nonEmpty;
61-
62-
MutableDefaultValue() { nonEmpty = mutableDefaultValue(this.(NameNode).getNode()) }
63-
64-
override string toString() { result = "mutable default value" }
65-
66-
override predicate isSourceOf(TaintKind kind) {
67-
nonEmpty = false and kind instanceof EmptyMutableValue
68-
or
69-
nonEmpty = true and kind instanceof NonEmptyMutableValue
70-
}
71-
}
72-
73-
private ClassValue mutable_class() {
74-
result = Value::named("list") or
75-
result = Value::named("dict")
76-
}
77-
78-
class Mutation extends TaintSink {
79-
Mutation() {
80-
exists(AugAssign a | a.getTarget().getAFlowNode() = this)
81-
or
82-
exists(Call c, Attribute a | c.getFunc() = a |
83-
a.getObject().getAFlowNode() = this and
84-
not safe_method(a.getName()) and
85-
this.(ControlFlowNode).pointsTo().getClass() = mutable_class()
86-
)
87-
}
88-
89-
override predicate sinks(TaintKind kind) {
90-
kind instanceof EmptyMutableValue
91-
or
92-
kind instanceof NonEmptyMutableValue
93-
}
94-
}
95-
96-
from TaintedPathSource src, TaintedPathSink sink
97-
where src.flowsTo(sink)
98-
select sink.getSink(), src, sink, "$@ flows to here and is mutated.", src.getSource(),
15+
import semmle.python.functions.ModificationOfParameterWithDefault
16+
import DataFlow::PathGraph
17+
18+
from
19+
ModificationOfParameterWithDefault::Configuration config, DataFlow::PathNode source,
20+
DataFlow::PathNode sink
21+
where config.hasFlowPath(source, sink)
22+
select sink.getNode(), source, sink, "$@ flows to here and is mutated.", source.getNode(),
9923
"Default value"

‎python/ql/src/semmle/python/functions/ModificationOfParameterWithDefault.qll

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/**
2+
* Provides a data-flow configuration for detecting modifications of a parameters default value.
3+
*
4+
* Note, for performance reasons: only import this file if
5+
* `ModificationOfParameterWithDefault::Configuration` is needed, otherwise
6+
* `ModificationOfParameterWithDefaultCustomizations` should be imported instead.
7+
*/
8+
9+
private import python
10+
import semmle.python.dataflow.new.DataFlow
11+
12+
/**
13+
* Provides a data-flow configuration for detecting modifications of a parameters default value.
14+
*/
15+
module ModificationOfParameterWithDefault {
16+
import ModificationOfParameterWithDefaultCustomizations::ModificationOfParameterWithDefault
17+
18+
/**
19+
* A data-flow configuration for detecting modifications of a parameters default value.
20+
*/
21+
class Configuration extends DataFlow::Configuration {
22+
Configuration() { this = "ModificationOfParameterWithDefault" }
23+
24+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
25+
26+
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
27+
28+
override predicate isBarrier(DataFlow::Node node) { node instanceof Barrier }
29+
30+
override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
31+
guard instanceof BarrierGuard
32+
}
33+
}
34+
}

‎python/ql/src/semmle/python/functions/ModificationOfParameterWithDefaultCustomizations.qll

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for detecting
3+
* modifications of a parameters default value, as well as extension points for adding your own.
4+
*/
5+
6+
private import python
7+
private import semmle.python.dataflow.new.DataFlow
8+
private import semmle.python.dataflow.new.BarrierGuards
9+
10+
/**
11+
* Provides default sources, sinks and sanitizers for detecting
12+
* "command injection"
13+
* vulnerabilities, as well as extension points for adding your own.
14+
*/
15+
module ModificationOfParameterWithDefault {
16+
/**
17+
* A data flow source for detecting modifications of a parameters default value.
18+
*/
19+
abstract class Source extends DataFlow::Node { }
20+
21+
/**
22+
* A data flow sink for detecting modifications of a parameters default value.
23+
*/
24+
abstract class Sink extends DataFlow::Node { }
25+
26+
/**
27+
* A sanitizer for detecting modifications of a parameters default value.
28+
*/
29+
abstract class Barrier extends DataFlow::Node { }
30+
31+
/**
32+
* A sanitizer guard for detecting modifications of a parameters default value.
33+
*/
34+
abstract class BarrierGuard extends DataFlow::BarrierGuard { }
35+
36+
/** Gets the truthiness (non emptyness) of the default of `p` if that value is mutable */
37+
private boolean mutableDefaultValue(Parameter p) {
38+
exists(Dict d | p.getDefault() = d |
39+
exists(d.getAKey()) and result = true
40+
or
41+
not exists(d.getAKey()) and result = false
42+
)
43+
or
44+
exists(List l | p.getDefault() = l |
45+
exists(l.getAnElt()) and result = true
46+
or
47+
not exists(l.getAnElt()) and result = false
48+
)
49+
}
50+
51+
/**
52+
* A source of remote user input, considered as a flow source.
53+
*/
54+
class MutableDefaultValue extends Source {
55+
boolean nonEmpty;
56+
57+
MutableDefaultValue() { nonEmpty = mutableDefaultValue(this.asCfgNode().(NameNode).getNode()) }
58+
}
59+
60+
predicate safe_method(string name) {
61+
name = "count" or
62+
name = "index" or
63+
name = "copy" or
64+
name = "get" or
65+
name = "has_key" or
66+
name = "items" or
67+
name = "keys" or
68+
name = "values" or
69+
name = "iteritems" or
70+
name = "iterkeys" or
71+
name = "itervalues" or
72+
name = "__contains__" or
73+
name = "__getitem__" or
74+
name = "__getattribute__"
75+
}
76+
77+
/**
78+
* A mutation is considered a flow sink.
79+
*/
80+
class Mutation extends Sink {
81+
Mutation() {
82+
exists(AugAssign a | a.getTarget().getAFlowNode() = this.asCfgNode())
83+
or
84+
exists(Call c, Attribute a | c.getFunc() = a |
85+
a.getObject().getAFlowNode() = this.asCfgNode() and
86+
not safe_method(a.getName())
87+
)
88+
}
89+
}
90+
}

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

Lines changed: 37 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,39 @@
11
edges
2-
| test.py:2:12:2:12 | empty mutable value | test.py:3:5:3:5 | empty mutable value |
3-
| test.py:7:14:7:14 | empty mutable value | test.py:9:14:9:14 | empty mutable value |
4-
| test.py:9:5:9:15 | empty mutable value | test.py:10:5:10:5 | empty mutable value |
5-
| test.py:9:14:9:14 | empty mutable value | test.py:9:5:9:15 | empty mutable value |
6-
| test.py:13:13:13:13 | empty mutable value | test.py:14:5:14:5 | empty mutable value |
7-
| test.py:18:14:18:14 | empty mutable value | test.py:19:13:19:13 | empty mutable value |
8-
| test.py:19:13:19:13 | empty mutable value | test.py:13:13:13:13 | empty mutable value |
9-
| test.py:23:14:23:14 | non-empty mutable value | test.py:24:5:24:5 | non-empty mutable value |
10-
| test.py:52:17:52:17 | empty mutable value | test.py:53:5:53:5 | empty mutable value |
11-
| test.py:57:26:57:26 | non-empty mutable value | test.py:58:5:58:5 | non-empty mutable value |
12-
| test.py:62:35:62:35 | non-empty mutable value | test.py:63:5:63:5 | non-empty mutable value |
13-
| test.py:66:21:66:21 | empty mutable value | test.py:67:5:67:5 | empty mutable value |
14-
| test.py:71:26:71:26 | empty mutable value | test.py:72:21:72:21 | empty mutable value |
15-
| test.py:72:21:72:21 | empty mutable value | test.py:66:21:66:21 | empty mutable value |
16-
| test.py:76:19:76:19 | empty mutable value | test.py:78:14:78:14 | empty mutable value |
17-
| test.py:78:5:78:15 | empty mutable value | test.py:79:5:79:5 | empty mutable value |
18-
| test.py:78:14:78:14 | empty mutable value | test.py:78:5:78:15 | empty mutable value |
2+
| test.py:2:12:2:12 | ControlFlowNode for l | test.py:3:5:3:5 | ControlFlowNode for l |
3+
| test.py:13:13:13:13 | ControlFlowNode for l | test.py:14:5:14:5 | ControlFlowNode for l |
4+
| test.py:18:14:18:14 | ControlFlowNode for l | test.py:19:13:19:13 | ControlFlowNode for l |
5+
| test.py:19:13:19:13 | ControlFlowNode for l | test.py:13:13:13:13 | ControlFlowNode for l |
6+
| test.py:23:14:23:14 | ControlFlowNode for l | test.py:24:5:24:5 | ControlFlowNode for l |
7+
| test.py:52:17:52:17 | ControlFlowNode for d | test.py:53:5:53:5 | ControlFlowNode for d |
8+
| test.py:57:26:57:26 | ControlFlowNode for d | test.py:58:5:58:5 | ControlFlowNode for d |
9+
| test.py:62:35:62:35 | ControlFlowNode for d | test.py:63:5:63:5 | ControlFlowNode for d |
10+
| test.py:66:21:66:21 | ControlFlowNode for d | test.py:67:5:67:5 | ControlFlowNode for d |
11+
| test.py:71:26:71:26 | ControlFlowNode for d | test.py:72:21:72:21 | ControlFlowNode for d |
12+
| test.py:72:21:72:21 | ControlFlowNode for d | test.py:66:21:66:21 | ControlFlowNode for d |
13+
nodes
14+
| test.py:2:12:2:12 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
15+
| test.py:3:5:3:5 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
16+
| test.py:13:13:13:13 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
17+
| test.py:14:5:14:5 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
18+
| test.py:18:14:18:14 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
19+
| test.py:19:13:19:13 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
20+
| test.py:23:14:23:14 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
21+
| test.py:24:5:24:5 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
22+
| test.py:52:17:52:17 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
23+
| test.py:53:5:53:5 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
24+
| test.py:57:26:57:26 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
25+
| test.py:58:5:58:5 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
26+
| test.py:62:35:62:35 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
27+
| test.py:63:5:63:5 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
28+
| test.py:66:21:66:21 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
29+
| test.py:67:5:67:5 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
30+
| test.py:71:26:71:26 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
31+
| test.py:72:21:72:21 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
1932
#select
20-
| test.py:3:5:3:5 | l | test.py:2:12:2:12 | empty mutable value | test.py:3:5:3:5 | empty mutable value | $@ flows to here and is mutated. | test.py:2:12:2:12 | l | Default value |
21-
| test.py:10:5:10:5 | x | test.py:7:14:7:14 | empty mutable value | test.py:10:5:10:5 | empty mutable value | $@ flows to here and is mutated. | test.py:7:14:7:14 | l | Default value |
22-
| test.py:14:5:14:5 | l | test.py:18:14:18:14 | empty mutable value | test.py:14:5:14:5 | empty mutable value | $@ flows to here and is mutated. | test.py:18:14:18:14 | l | Default value |
23-
| test.py:24:5:24:5 | l | test.py:23:14:23:14 | non-empty mutable value | test.py:24:5:24:5 | non-empty mutable value | $@ flows to here and is mutated. | test.py:23:14:23:14 | l | Default value |
24-
| test.py:53:5:53:5 | d | test.py:52:17:52:17 | empty mutable value | test.py:53:5:53:5 | empty mutable value | $@ flows to here and is mutated. | test.py:52:17:52:17 | d | Default value |
25-
| test.py:58:5:58:5 | d | test.py:57:26:57:26 | non-empty mutable value | test.py:58:5:58:5 | non-empty mutable value | $@ flows to here and is mutated. | test.py:57:26:57:26 | d | Default value |
26-
| test.py:63:5:63:5 | d | test.py:62:35:62:35 | non-empty mutable value | test.py:63:5:63:5 | non-empty mutable value | $@ flows to here and is mutated. | test.py:62:35:62:35 | d | Default value |
27-
| test.py:67:5:67:5 | d | test.py:71:26:71:26 | empty mutable value | test.py:67:5:67:5 | empty mutable value | $@ flows to here and is mutated. | test.py:71:26:71:26 | d | Default value |
28-
| test.py:79:5:79:5 | x | test.py:76:19:76:19 | empty mutable value | test.py:79:5:79:5 | empty mutable value | $@ flows to here and is mutated. | test.py:76:19:76:19 | d | Default value |
33+
| test.py:3:5:3:5 | ControlFlowNode for l | test.py:2:12:2:12 | ControlFlowNode for l | test.py:3:5:3:5 | ControlFlowNode for l | $@ flows to here and is mutated. | test.py:2:12:2:12 | ControlFlowNode for l | Default value |
34+
| test.py:14:5:14:5 | ControlFlowNode for l | test.py:18:14:18:14 | ControlFlowNode for l | test.py:14:5:14:5 | ControlFlowNode for l | $@ flows to here and is mutated. | test.py:18:14:18:14 | ControlFlowNode for l | Default value |
35+
| test.py:24:5:24:5 | ControlFlowNode for l | test.py:23:14:23:14 | ControlFlowNode for l | test.py:24:5:24:5 | ControlFlowNode for l | $@ flows to here and is mutated. | test.py:23:14:23:14 | ControlFlowNode for l | Default value |
36+
| test.py:53:5:53:5 | ControlFlowNode for d | test.py:52:17:52:17 | ControlFlowNode for d | test.py:53:5:53:5 | ControlFlowNode for d | $@ flows to here and is mutated. | test.py:52:17:52:17 | ControlFlowNode for d | Default value |
37+
| test.py:58:5:58:5 | ControlFlowNode for d | test.py:57:26:57:26 | ControlFlowNode for d | test.py:58:5:58:5 | ControlFlowNode for d | $@ flows to here and is mutated. | test.py:57:26:57:26 | ControlFlowNode for d | Default value |
38+
| test.py:63:5:63:5 | ControlFlowNode for d | test.py:62:35:62:35 | ControlFlowNode for d | test.py:63:5:63:5 | ControlFlowNode for d | $@ flows to here and is mutated. | test.py:62:35:62:35 | ControlFlowNode for d | Default value |
39+
| test.py:67:5:67:5 | ControlFlowNode for d | test.py:71:26:71:26 | ControlFlowNode for d | test.py:67:5:67:5 | ControlFlowNode for d | $@ flows to here and is mutated. | test.py:71:26:71:26 | ControlFlowNode for d | Default value |

‎python/ql/test/query-tests/Functions/ModificationOfParameterWithDefault/test.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ def simple(l = []):
77
def includes(l = []):
88
x = [0]
99
x.extend(l)
10-
x.extend([1]) # FP
10+
x.extend([1])
1111
return x
1212

1313
def extends(l):
@@ -76,5 +76,5 @@ def dict_deferred_method(d = {}):
7676
def dict_includes(d = {}):
7777
x = {}
7878
x.update(d)
79-
x.update({'a': 1}) # FP
79+
x.update({'a': 1})
8080
return x

0 commit comments

Comments
 (0)