Skip to content

Commit a30b456

Browse files
taus-semmletaus-semmle
authored
Merge pull request #1020 from markshannon/python-taint-tracking-guard
Python: Add taint tracking guard for truthiness.
2 parents 8a16164 + 94190e7 commit a30b456

File tree

10 files changed

+105
-37
lines changed

10 files changed

+105
-37
lines changed

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

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,40 +19,57 @@ predicate safe_method(string name) {
1919
name = "items" or name = "keys" or name = "values" or name = "iteritems" or name = "iterkeys" or name = "itervalues"
2020
}
2121

22-
predicate maybe_parameter(SsaVariable var, Function f, Parameter p) {
23-
p = var.getAnUltimateDefinition().getDefinition().getNode() and
24-
f.getAnArg() = p
22+
/** Gets the truthiness (non emptyness) of the default of `p` if that value is mutable */
23+
private boolean mutableDefaultValue(Parameter p) {
24+
exists(Dict d |
25+
p.getDefault() = d |
26+
exists(d.getAKey()) and result = true
27+
or
28+
not exists(d.getAKey()) and result = false
29+
)
30+
or
31+
exists(List l |
32+
p.getDefault() = l |
33+
exists(l.getAnElt()) and result = true
34+
or
35+
not exists(l.getAnElt()) and result = false
36+
)
2537
}
2638

27-
predicate has_mutable_default(Parameter p) {
28-
exists(SsaVariable v, FunctionExpr f | maybe_parameter(v, f.getInnerScope(), p) and
29-
exists(int i, int def_cnt, int arg_cnt |
30-
def_cnt = count(f.getArgs().getADefault()) and
31-
arg_cnt = count(f.getInnerScope().getAnArg()) and
32-
i in [1 .. arg_cnt] and
33-
(f.getArgs().getDefault(def_cnt - i) instanceof Dict or f.getArgs().getDefault(def_cnt - i) instanceof List) and
34-
f.getInnerScope().getArgName(arg_cnt - i) = v.getId()
35-
)
36-
)
39+
40+
class NonEmptyMutableValue extends TaintKind {
41+
NonEmptyMutableValue() {
42+
this = "non-empty mutable value"
43+
}
3744
}
3845

39-
class MutableValue extends TaintKind {
40-
MutableValue() {
41-
this = "mutable value"
46+
class EmptyMutableValue extends TaintKind {
47+
EmptyMutableValue() {
48+
this = "empty mutable value"
4249
}
50+
51+
override boolean booleanValue() {
52+
result = false
53+
}
54+
4355
}
4456

4557
class MutableDefaultValue extends TaintSource {
58+
59+
boolean nonEmpty;
60+
4661
MutableDefaultValue() {
47-
has_mutable_default(this.(NameNode).getNode())
62+
nonEmpty = mutableDefaultValue(this.(NameNode).getNode())
4863
}
4964

5065
override string toString() {
5166
result = "mutable default value"
5267
}
5368

5469
override predicate isSourceOf(TaintKind kind) {
55-
kind instanceof MutableValue
70+
nonEmpty = false and kind instanceof EmptyMutableValue
71+
or
72+
nonEmpty = true and kind instanceof NonEmptyMutableValue
5673
}
5774
}
5875

@@ -68,7 +85,9 @@ class Mutation extends TaintSink {
6885
}
6986

7087
override predicate sinks(TaintKind kind) {
71-
kind instanceof MutableValue
88+
kind instanceof EmptyMutableValue
89+
or
90+
kind instanceof NonEmptyMutableValue
7291
}
7392
}
7493

‎python/ql/src/semmle/python/security/TaintTracking.qll‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,16 @@ abstract class TaintKind extends string {
148148
none()
149149
}
150150

151+
/** Gets the boolean values (may be one, neither, or both) that
152+
* may result from the Python expression `bool(this)`
153+
*/
154+
boolean booleanValue() {
155+
/* Default to true as the vast majority of taint is strings and
156+
* the empty string is almost always benign.
157+
*/
158+
result = true
159+
}
160+
151161
string repr() { result = this }
152162

153163
}
@@ -1190,7 +1200,8 @@ library module TaintFlowImplementation {
11901200
sanitizer.sanitizingEdge(kind, test)
11911201
)
11921202
|
1193-
not Filters::isinstance(test.getTest(), _, var.getSourceVariable().getAUse())
1203+
not Filters::isinstance(test.getTest(), _, var.getSourceVariable().getAUse()) and
1204+
not test.getTest() = var.getSourceVariable().getAUse()
11941205
or
11951206
exists(ControlFlowNode c, ClassObject cls |
11961207
Filters::isinstance(test.getTest(), c, var.getSourceVariable().getAUse())
@@ -1200,6 +1211,8 @@ library module TaintFlowImplementation {
12001211
or
12011212
test.getSense() = false and not kind.getClass().getAnImproperSuperType() = cls
12021213
)
1214+
or
1215+
test.getTest() = var.getSourceVariable().getAUse() and kind.booleanValue() = test.getSense()
12031216
)
12041217
}
12051218

‎python/ql/test/library-tests/taint/general/TestNode.expected‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,11 @@
215215
| Taint simple.test | test.py:169 | SOURCE | |
216216
| Taint simple.test | test.py:172 | Subscript | |
217217
| Taint simple.test | test.py:173 | Subscript | |
218+
| Taint simple.test | test.py:178 | SOURCE | |
219+
| Taint simple.test | test.py:179 | t | |
220+
| Taint simple.test | test.py:180 | t | |
221+
| Taint simple.test | test.py:183 | t | |
222+
| Taint simple.test | test.py:186 | t | |
218223
| Taint {simple.test} | test.py:169 | Dict | |
219224
| Taint {simple.test} | test.py:171 | d | |
220225
| Taint {simple.test} | test.py:173 | y | |

‎python/ql/test/library-tests/taint/general/TestSink.expected‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,5 @@
3232
| simple.test | test.py:159 | 160 | t | simple.test |
3333
| simple.test | test.py:168 | 172 | Subscript | simple.test |
3434
| simple.test | test.py:169 | 173 | Subscript | simple.test |
35+
| simple.test | test.py:178 | 180 | t | simple.test |
36+
| simple.test | test.py:178 | 186 | t | simple.test |

‎python/ql/test/library-tests/taint/general/TestSource.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,4 @@
4040
| test.py:163 | SOURCE | simple.test |
4141
| test.py:168 | SOURCE | simple.test |
4242
| test.py:169 | SOURCE | simple.test |
43+
| test.py:178 | SOURCE | simple.test |

‎python/ql/test/library-tests/taint/general/TestStep.expected‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,10 @@
173173
| Taint simple.test | test.py:163 | SOURCE | | --> | Taint simple.test | test.py:164 | s | |
174174
| Taint simple.test | test.py:168 | SOURCE | | --> | Taint [simple.test] | test.py:168 | List | |
175175
| Taint simple.test | test.py:169 | SOURCE | | --> | Taint {simple.test} | test.py:169 | Dict | |
176+
| Taint simple.test | test.py:178 | SOURCE | | --> | Taint simple.test | test.py:179 | t | |
177+
| Taint simple.test | test.py:178 | SOURCE | | --> | Taint simple.test | test.py:180 | t | |
178+
| Taint simple.test | test.py:178 | SOURCE | | --> | Taint simple.test | test.py:183 | t | |
179+
| Taint simple.test | test.py:178 | SOURCE | | --> | Taint simple.test | test.py:186 | t | |
176180
| Taint {simple.test} | test.py:169 | Dict | | --> | Taint {simple.test} | test.py:171 | d | |
177181
| Taint {simple.test} | test.py:169 | Dict | | --> | Taint {simple.test} | test.py:175 | d | |
178182
| Taint {simple.test} | test.py:171 | d | | --> | Taint {simple.test} | test.py:173 | y | |

‎python/ql/test/library-tests/taint/general/TestVar.expected‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,3 +177,8 @@
177177
| test.py:174 | l_2 | test.py:168 | Taint [simple.test] | List |
178178
| test.py:175 | d2_0 | test.py:175 | Taint {simple.test} | dict() |
179179
| test.py:175 | d_2 | test.py:169 | Taint {simple.test} | Dict |
180+
| test.py:178 | t_0 | test.py:178 | Taint simple.test | SOURCE |
181+
| test.py:180 | t_1 | test.py:178 | Taint simple.test | SOURCE |
182+
| test.py:180 | t_2 | test.py:178 | Taint simple.test | SOURCE |
183+
| test.py:183 | t_3 | test.py:178 | Taint simple.test | SOURCE |
184+
| test.py:186 | t_4 | test.py:178 | Taint simple.test | SOURCE |

‎python/ql/test/library-tests/taint/general/test.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,3 +173,14 @@ def test_update_extend(x, y):
173173
SINK(y["key"])
174174
l2 = list(l)
175175
d2 = dict(d)
176+
177+
def test_truth():
178+
t = SOURCE
179+
if t:
180+
SINK(t)
181+
else:
182+
SINK(t)
183+
if not t:
184+
SINK(t)
185+
else:
186+
SINK(t)

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

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,22 @@
11
edges
2-
| functions_test.py:36:9:36:9 | mutable value | functions_test.py:37:16:37:16 | mutable value |
3-
| functions_test.py:39:9:39:9 | mutable value | functions_test.py:40:5:40:5 | mutable value |
4-
| functions_test.py:238:15:238:15 | mutable value | functions_test.py:239:5:239:5 | mutable value |
5-
| functions_test.py:290:25:290:25 | mutable value | functions_test.py:291:5:291:5 | mutable value |
6-
| functions_test.py:293:21:293:21 | mutable value | functions_test.py:294:5:294:5 | mutable value |
7-
| functions_test.py:296:27:296:27 | mutable value | functions_test.py:297:25:297:25 | mutable value |
8-
| functions_test.py:296:27:296:27 | mutable value | functions_test.py:298:21:298:21 | mutable value |
9-
| functions_test.py:297:25:297:25 | mutable value | functions_test.py:290:25:290:25 | mutable value |
10-
| functions_test.py:298:21:298:21 | mutable value | functions_test.py:293:21:293:21 | mutable value |
2+
| functions_test.py:36:9:36:9 | empty mutable value | functions_test.py:37:16:37:16 | empty mutable value |
3+
| functions_test.py:39:9:39:9 | empty mutable value | functions_test.py:40:5:40:5 | empty mutable value |
4+
| functions_test.py:238:15:238:15 | empty mutable value | functions_test.py:239:5:239:5 | empty mutable value |
5+
| functions_test.py:290:25:290:25 | empty mutable value | functions_test.py:291:5:291:5 | empty mutable value |
6+
| functions_test.py:293:21:293:21 | empty mutable value | functions_test.py:294:5:294:5 | empty mutable value |
7+
| functions_test.py:296:27:296:27 | empty mutable value | functions_test.py:297:25:297:25 | empty mutable value |
8+
| functions_test.py:296:27:296:27 | empty mutable value | functions_test.py:298:21:298:21 | empty mutable value |
9+
| functions_test.py:297:25:297:25 | empty mutable value | functions_test.py:290:25:290:25 | empty mutable value |
10+
| functions_test.py:298:21:298:21 | empty mutable value | functions_test.py:293:21:293:21 | empty mutable value |
11+
| functions_test.py:300:26:300:26 | empty mutable value | functions_test.py:301:8:301:8 | empty mutable value |
12+
| functions_test.py:300:26:300:26 | empty mutable value | functions_test.py:303:12:303:12 | empty mutable value |
1113
parents
12-
| functions_test.py:290:25:290:25 | mutable value | functions_test.py:297:25:297:25 | mutable value |
13-
| functions_test.py:291:5:291:5 | mutable value | functions_test.py:297:25:297:25 | mutable value |
14-
| functions_test.py:293:21:293:21 | mutable value | functions_test.py:298:21:298:21 | mutable value |
15-
| functions_test.py:294:5:294:5 | mutable value | functions_test.py:298:21:298:21 | mutable value |
14+
| functions_test.py:290:25:290:25 | empty mutable value | functions_test.py:297:25:297:25 | empty mutable value |
15+
| functions_test.py:291:5:291:5 | empty mutable value | functions_test.py:297:25:297:25 | empty mutable value |
16+
| functions_test.py:293:21:293:21 | empty mutable value | functions_test.py:298:21:298:21 | empty mutable value |
17+
| functions_test.py:294:5:294:5 | empty mutable value | functions_test.py:298:21:298:21 | empty mutable value |
1618
#select
17-
| 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 |
18-
| 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 |
19-
| 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 |
20-
| 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 |
19+
| 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 |
20+
| 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 |
21+
| 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 |
22+
| 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 |

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,3 +296,9 @@ def mutate_argument(x):
296296
def indirect_modification(y = []):
297297
aug_assign_argument(y)
298298
mutate_argument(y)
299+
300+
def guarded_modification(z=[]):
301+
if z:
302+
z.append(0)
303+
return z
304+

0 commit comments

Comments
 (0)