Skip to content

Python: Add taint tracking guard for truthiness. #1020

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
taus-semmle merged 2 commits into from
Mar 1, 2019
Merged

Python: Add taint tracking guard for truthiness. #1020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ebd9bc3
Python: Improve taint tracking to account for truthiness of the taint…
markshannon Mar 1, 2019
94190e7
Python: Update py/modification-of-default-value to account for truthi…
markshannon Mar 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 38 additions & 19 deletions python/ql/src/Functions/ModificationOfParameterWithDefault.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,40 +19,57 @@ 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
/** 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
)
}

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()
)
)

class NonEmptyMutableValue extends TaintKind {
NonEmptyMutableValue() {
this = "non-empty mutable value"
}
}

class MutableValue extends TaintKind {
MutableValue() {
this = "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() {
result = "mutable default value"
}

override predicate isSourceOf(TaintKind kind) {
kind instanceof MutableValue
nonEmpty = false and kind instanceof EmptyMutableValue
or
nonEmpty = true and kind instanceof NonEmptyMutableValue
}
}

Expand All @@ -68,7 +85,9 @@ class Mutation extends TaintSink {
}

override predicate sinks(TaintKind kind) {
kind instanceof MutableValue
kind instanceof EmptyMutableValue
or
kind instanceof NonEmptyMutableValue
}
}

Expand Down
15 changes: 14 additions & 1 deletion python/ql/src/semmle/python/security/TaintTracking.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,16 @@ abstract class TaintKind extends string {
none()
}

/** Gets the boolean values (may be one, neither, or both) that
* may result from the Python expression `bool(this)`
*/
boolean booleanValue() {
/* Default to true as the vast majority of taint is strings and
* the empty string is almost always benign.
*/
result = true
}

string repr() { result = this }

}
Expand Down Expand Up @@ -1190,7 +1200,8 @@ library module TaintFlowImplementation {
sanitizer.sanitizingEdge(kind, test)
)
|
not Filters::isinstance(test.getTest(), _, var.getSourceVariable().getAUse())
not Filters::isinstance(test.getTest(), _, var.getSourceVariable().getAUse()) and
not test.getTest() = var.getSourceVariable().getAUse()
or
exists(ControlFlowNode c, ClassObject cls |
Filters::isinstance(test.getTest(), c, var.getSourceVariable().getAUse())
Expand All @@ -1200,6 +1211,8 @@ library module TaintFlowImplementation {
or
test.getSense() = false and not kind.getClass().getAnImproperSuperType() = cls
)
or
test.getTest() = var.getSourceVariable().getAUse() and kind.booleanValue() = test.getSense()
)
}

Expand Down
5 changes: 5 additions & 0 deletions python/ql/test/library-tests/taint/general/TestNode.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,11 @@
| Taint simple.test | test.py:169 | SOURCE | |
| Taint simple.test | test.py:172 | Subscript | |
| Taint simple.test | test.py:173 | Subscript | |
| Taint simple.test | test.py:178 | SOURCE | |
| Taint simple.test | test.py:179 | t | |
| Taint simple.test | test.py:180 | t | |
| Taint simple.test | test.py:183 | t | |
| Taint simple.test | test.py:186 | t | |
| Taint {simple.test} | test.py:169 | Dict | |
| Taint {simple.test} | test.py:171 | d | |
| Taint {simple.test} | test.py:173 | y | |
Expand Down
2 changes: 2 additions & 0 deletions python/ql/test/library-tests/taint/general/TestSink.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,5 @@
| simple.test | test.py:159 | 160 | t | simple.test |
| simple.test | test.py:168 | 172 | Subscript | simple.test |
| simple.test | test.py:169 | 173 | Subscript | simple.test |
| simple.test | test.py:178 | 180 | t | simple.test |
| simple.test | test.py:178 | 186 | t | simple.test |
1 change: 1 addition & 0 deletions python/ql/test/library-tests/taint/general/TestSource.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,4 @@
| test.py:163 | SOURCE | simple.test |
| test.py:168 | SOURCE | simple.test |
| test.py:169 | SOURCE | simple.test |
| test.py:178 | SOURCE | simple.test |
4 changes: 4 additions & 0 deletions python/ql/test/library-tests/taint/general/TestStep.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@
| Taint simple.test | test.py:163 | SOURCE | | --> | Taint simple.test | test.py:164 | s | |
| Taint simple.test | test.py:168 | SOURCE | | --> | Taint [simple.test] | test.py:168 | List | |
| Taint simple.test | test.py:169 | SOURCE | | --> | Taint {simple.test} | test.py:169 | Dict | |
| Taint simple.test | test.py:178 | SOURCE | | --> | Taint simple.test | test.py:179 | t | |
| Taint simple.test | test.py:178 | SOURCE | | --> | Taint simple.test | test.py:180 | t | |
| Taint simple.test | test.py:178 | SOURCE | | --> | Taint simple.test | test.py:183 | t | |
| Taint simple.test | test.py:178 | SOURCE | | --> | Taint simple.test | test.py:186 | t | |
| Taint {simple.test} | test.py:169 | Dict | | --> | Taint {simple.test} | test.py:171 | d | |
| Taint {simple.test} | test.py:169 | Dict | | --> | Taint {simple.test} | test.py:175 | d | |
| Taint {simple.test} | test.py:171 | d | | --> | Taint {simple.test} | test.py:173 | y | |
Expand Down
5 changes: 5 additions & 0 deletions python/ql/test/library-tests/taint/general/TestVar.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,3 +177,8 @@
| test.py:174 | l_2 | test.py:168 | Taint [simple.test] | List |
| test.py:175 | d2_0 | test.py:175 | Taint {simple.test} | dict() |
| test.py:175 | d_2 | test.py:169 | Taint {simple.test} | Dict |
| test.py:178 | t_0 | test.py:178 | Taint simple.test | SOURCE |
| test.py:180 | t_1 | test.py:178 | Taint simple.test | SOURCE |
| test.py:180 | t_2 | test.py:178 | Taint simple.test | SOURCE |
| test.py:183 | t_3 | test.py:178 | Taint simple.test | SOURCE |
| test.py:186 | t_4 | test.py:178 | Taint simple.test | SOURCE |
11 changes: 11 additions & 0 deletions python/ql/test/library-tests/taint/general/test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,3 +173,14 @@ def test_update_extend(x, y):
SINK(y["key"])
l2 = list(l)
d2 = dict(d)

def test_truth():
t = SOURCE
if t:
SINK(t)
else:
SINK(t)
if not t:
SINK(t)
else:
SINK(t)
36 changes: 19 additions & 17 deletions python/ql/test/query-tests/Functions/general/ModificationOfParameterWithDefault.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 |
6 changes: 6 additions & 0 deletions python/ql/test/query-tests/Functions/general/functions_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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