Skip to content

Commit 3f80668

Browse files
markshannonmarkshannon
committed
Python taint-tracking: Fix up handling of contexts for __init__ and for context-free taints.
1 parent fe9c9d4 commit 3f80668

10 files changed

Lines changed: 318 additions & 277 deletions

File tree

‎python/ql/src/semmle/python/dataflow/Implementation.qll‎

Lines changed: 71 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@ newtype TTaintTrackingContext =
1212

1313
class TaintTrackingContext extends TTaintTrackingContext {
1414

15-
string toString() {
15+
string toString() {
1616
this = TNoParam() and result = ""
1717
or
1818
exists(TaintKind param, AttributePath path, int n |
1919
this = TParamContext(param, path, n) and
20-
result = "Parameter " + n.toString() + "(" + path.toString() + ") is " + param
20+
result = "p" + n.toString() + path.extension() + " = " + param
2121
)
2222
}
2323

@@ -74,7 +74,7 @@ private newtype TAttributePath =
7474
TNoAttribute()
7575
or
7676
TAttribute(string name) {
77-
none()
77+
exists(Attribute a | a.getName() = name)
7878
}
7979
or
8080
TAttributeAttribute(string name1, string name2) {
@@ -114,15 +114,18 @@ class NoAttribute extends TNoAttribute, AttributePath {
114114

115115
class NamedAttributePath extends TAttribute, AttributePath {
116116

117-
override string toString() {
117+
override string toString() {
118118
exists(string attr |
119119
this = TAttribute(attr) and
120120
result = "attribute " + attr
121121
)
122122
}
123123

124-
override string extension() {
125-
this = TAttribute(result)
124+
override string extension() {
125+
exists(string attr |
126+
this = TAttribute(attr) and
127+
result = "." + attr
128+
)
126129
}
127130

128131
override AttributePath fromAttribute(string name) {
@@ -140,7 +143,13 @@ newtype TTaintTrackingNode =
140143

141144
class TaintTrackingNode extends TTaintTrackingNode {
142145

143-
string toString() { result = this.getTaintKind().repr() }
146+
string toString() {
147+
if this.getPath() instanceof NoAttribute then (
148+
result = this.getTaintKind().repr()
149+
) else (
150+
result = this.getPath().extension() + " = " + this.getTaintKind().repr()
151+
)
152+
}
144153

145154
DataFlow::Node getNode() {
146155
this = TTaintTrackingNode_(result, _, _, _, _)
@@ -398,6 +407,8 @@ class TaintTrackingImplementation extends string {
398407
or
399408
this.essaFlowStep(src, node, context, path, kind) and edgeLabel = ""
400409
or
410+
this.instantionStep(src, node, context, path, kind, edgeLabel)
411+
or
401412
this.legacyExtensionStep(src, node, context, path, kind, edgeLabel)
402413
or
403414
exists(DataFlow::Node srcnode, TaintKind srckind |
@@ -425,11 +436,11 @@ class TaintTrackingImplementation extends string {
425436
pragma [noinline]
426437
predicate importStep(TaintTrackingNode src, DataFlow::Node node, TaintTrackingContext context, AttributePath path, TaintKind kind, string edgeLabel) {
427438
edgeLabel = "import" and
428-
exists(ModuleValue m, string name |
429-
src = TTaintTrackingNode_(_, context, TNoAttribute(), kind, this) and
439+
exists(ModuleValue m, string name, AttributePath srcpath |
440+
src = TTaintTrackingNode_(_, context, srcpath, kind, this) and
430441
this.moduleAttributeTainted(m, name, src) and
431-
node.asCfgNode().(ImportExprNode).pointsTo(m) and
432-
path = TAttribute(name)
442+
node.asCfgNode().pointsTo(m) and
443+
path = srcpath.getAttribute(name)
433444
)
434445
}
435446

@@ -454,7 +465,8 @@ class TaintTrackingImplementation extends string {
454465
exists(DataFlow::Node srcnode, TaintKind srckind, string attrname |
455466
src = TTaintTrackingNode_(srcnode, context, path, srckind, this) and
456467
srcnode.asCfgNode() = node.asCfgNode().(AttrNode).getObject(attrname) and
457-
kind = srckind.getTaintOfAttribute(attrname) and edgeLabel = "from taint attribute"
468+
kind = srckind.getTaintOfAttribute(attrname) and edgeLabel = "from taint attribute" and
469+
path instanceof NoAttribute
458470
)
459471
}
460472

@@ -498,16 +510,27 @@ class TaintTrackingImplementation extends string {
498510

499511
pragma [noinline]
500512
predicate returnFlowStep(TaintTrackingNode src, DataFlow::Node node, TaintTrackingContext context, AttributePath path, TaintKind kind, string edgeLabel) {
501-
exists(CallNode call, PythonFunctionObjectInternal pyfunc, int arg, TaintKind callerKind, DataFlow::Node srcNode, AttributePath callerPath, TaintTrackingContext srcContext |
502-
src = TTaintTrackingNode_(srcNode, srcContext, path, kind, this) and
503-
this.callWithTaintedArgument(_, call, context, pyfunc, arg, callerPath, callerKind) and
504-
srcContext = TParamContext(callerKind, callerPath, arg) and
513+
exists(CallNode call, PythonFunctionObjectInternal pyfunc, TaintTrackingContext callee, DataFlow::Node retval |
514+
this.callContexts(call, pyfunc, context, callee) and
515+
src = TTaintTrackingNode_(retval, callee, path, kind, this) and
505516
node.asCfgNode() = call and
506-
srcNode.asCfgNode() = any(Return ret | ret.getScope() = pyfunc.getScope()).getValue().getAFlowNode()
517+
retval.asCfgNode() = any(Return ret | ret.getScope() = pyfunc.getScope()).getValue().getAFlowNode()
507518
) and
508519
edgeLabel = "return"
509520
}
510521

522+
pragma [noinline]
523+
predicate callContexts(CallNode call, PythonFunctionObjectInternal pyfunc, TaintTrackingContext caller, TaintTrackingContext callee) {
524+
exists(int arg, TaintKind callerKind, AttributePath callerPath |
525+
this.callWithTaintedArgument(_, call, caller, pyfunc, arg, callerPath, callerKind) and
526+
callee = TParamContext(callerKind, callerPath, arg)
527+
)
528+
or
529+
pyfunc.getACall() = call and
530+
callee = TNoParam() and
531+
caller = TNoParam()
532+
}
533+
511534
predicate callWithTaintedArgument(TaintTrackingNode src, CallNode call, TaintTrackingContext caller, CallableValue pyfunc, int arg, AttributePath path, TaintKind kind) {
512535
exists(DataFlow::Node srcnode |
513536
src = TTaintTrackingNode_(srcnode, caller, path, kind, this) and
@@ -638,7 +661,7 @@ class TaintTrackingImplementation extends string {
638661
defn.getValue() = srcnode.asCfgNode() and
639662
defn.getName() = attrname and
640663
path = srcpath.getAttribute(attrname)
641-
)
664+
)
642665
}
643666

644667
pragma [noinline]
@@ -698,7 +721,6 @@ class TaintTrackingImplementation extends string {
698721
)
699722
}
700723

701-
702724
pragma [noinline]
703725
predicate taintedExceptionCapture(TaintTrackingNode src, ExceptionCapture defn, TaintTrackingContext context, AttributePath path, TaintKind kind) {
704726
exists(DataFlow::Node srcnode |
@@ -717,6 +739,36 @@ class TaintTrackingImplementation extends string {
717739
)
718740
}
719741

742+
predicate instantionStep(TaintTrackingNode src, DataFlow::Node node, TaintTrackingContext context, AttributePath path, TaintKind kind, string edgeLabel) {
743+
exists(DataFlow::Node srcnode, PythonFunctionValue init, EssaVariable self, TaintTrackingContext callee |
744+
instantionCall(node.asCfgNode(), init, context, callee) and
745+
src = TTaintTrackingNode_(srcnode, callee, path, kind, this) and
746+
srcnode.asVariable() = self and
747+
self.getSourceVariable().(Variable).isSelf() and
748+
BaseFlow::reaches_exit(self) and
749+
self.getScope() = init.getScope()
750+
) and
751+
edgeLabel = "instantiation"
752+
}
753+
754+
predicate instantionCall(CallNode call, PythonFunctionObjectInternal init, TaintTrackingContext caller, TaintTrackingContext callee) {
755+
exists(ClassValue cls |
756+
call.getFunction().pointsTo(cls) and
757+
cls.lookup("__init__") = init
758+
|
759+
exists(int arg, TaintKind callerKind, AttributePath callerPath |
760+
exists(TaintTrackingNode tainted, DataFlow::Node argument |
761+
tainted = TTaintTrackingNode_(argument, caller, callerPath, callerKind, this) and
762+
call.getArg(arg-1) = argument.asCfgNode() and
763+
callee = TParamContext(callerKind, callerPath, arg)
764+
)
765+
)
766+
or
767+
callee = TNoParam() and
768+
caller = TNoParam()
769+
)
770+
}
771+
720772
}
721773

722774
/* Backwards compatibility with config-less taint-tracking */

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

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1 @@
1-
import python
2-
3-
import semmle.python.security.TaintTracking
4-
private import semmle.python.dataflow.Implementation
5-
6-
private predicate sourceReaches(TaintTrackingNode node) {
7-
exists(TaintTrackingNode src |
8-
src.getConfiguration() = node.getConfiguration() and
9-
src.isSource() and src.getASuccessor*() = node
10-
)
11-
}
12-
13-
private predicate reachesSink(TaintTrackingNode node) {
14-
exists(TaintTrackingNode sink |
15-
sink.getConfiguration() = node.getConfiguration() and
16-
sink.isSink() and node.getASuccessor*() = sink
17-
)
18-
}
19-
20-
query predicate edges(TaintTrackingNode fromnode, TaintTrackingNode tonode) {
21-
sourceReaches(fromnode) and
22-
reachesSink(tonode) and
23-
fromnode.getASuccessor() = tonode
24-
}
1+
import semmle.python.dataflow.Paths

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

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,30 @@
11
WARNING: Type CallContext has been deprecated and may be removed in future (Contexts.ql:6,6-17)
22
WARNING: Type CallContext has been deprecated and may be removed in future (Contexts.ql:7,14-25)
3-
| Parameter 0(no attribute) is Command injection | Function isEscapedSql |
4-
| Parameter 0(no attribute) is Command injection | Function isValidCommand |
5-
| Parameter 0(no attribute) is SQL injection | Function isEscapedSql |
6-
| Parameter 0(no attribute) is SQL injection | Function isValidCommand |
7-
| Parameter 0(no attribute) is basic.custom | Function hub |
8-
| Parameter 0(no attribute) is explicit.carrier | Function hub |
9-
| Parameter 0(no attribute) is paper | Function paper |
10-
| Parameter 0(no attribute) is paper | Function scissors |
11-
| Parameter 0(no attribute) is rock | Function paper |
12-
| Parameter 0(no attribute) is scissors | Function paper |
13-
| Parameter 0(no attribute) is scissors | Function rock |
14-
| Parameter 0(no attribute) is scissors | Function scissors |
15-
| Parameter 0(no attribute) is simple.test | Function f1 |
16-
| Parameter 0(no attribute) is simple.test | Function f2 |
17-
| Parameter 0(no attribute) is simple.test | Function f3 |
18-
| Parameter 0(no attribute) is simple.test | Function f4 |
19-
| Parameter 0(no attribute) is simple.test | Function f5 |
20-
| Parameter 0(no attribute) is simple.test | Function f6 |
21-
| Parameter 0(no attribute) is simple.test | Function hub |
22-
| Parameter 0(no attribute) is simple.test | Function sink |
23-
| Parameter 1(no attribute) is explicit.carrier | Function __init__ |
24-
| Parameter 1(no attribute) is simple.test | Function __init__ |
25-
| Parameter 1(no attribute) is simple.test | Function sink3 |
3+
| p0 = Command injection | Function isEscapedSql |
4+
| p0 = Command injection | Function isValidCommand |
5+
| p0 = SQL injection | Function isEscapedSql |
6+
| p0 = SQL injection | Function isValidCommand |
7+
| p0 = basic.custom | Function hub |
8+
| p0 = explicit.carrier | Function hub |
9+
| p0 = paper | Function paper |
10+
| p0 = paper | Function scissors |
11+
| p0 = rock | Function paper |
12+
| p0 = scissors | Function paper |
13+
| p0 = scissors | Function rock |
14+
| p0 = scissors | Function scissors |
15+
| p0 = simple.test | Function f1 |
16+
| p0 = simple.test | Function f2 |
17+
| p0 = simple.test | Function f3 |
18+
| p0 = simple.test | Function f4 |
19+
| p0 = simple.test | Function f5 |
20+
| p0 = simple.test | Function f6 |
21+
| p0 = simple.test | Function hub |
22+
| p0 = simple.test | Function sink |
23+
| p0 = simple.test | Function sink2 |
24+
| p0.attr = simple.test | Function get_attr |
25+
| p0.attr = simple.test | Function hub |
26+
| p0.x = simple.test | Function hub |
27+
| p0.x = simple.test | Function x_sink |
28+
| p1 = explicit.carrier | Function __init__ |
29+
| p1 = simple.test | Function __init__ |
30+
| p1 = simple.test | Function sink3 |

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1-
| Module deep | x | Taint simple.test | | deep.py:20 |
2-
| Module module | dangerous | Taint simple.test | | module.py:3 |
3-
| Module test | unsafe | Taint simple.test | | test.py:156 |
1+
| Module deep | x | simple.test | | deep.py:20 |
2+
| Module module | dangerous | simple.test | | module.py:3 |
3+
| Module test | module | .dangerous = simple.test | | test.py:85 |
4+
| Module test | unsafe | simple.test | | test.py:156 |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@ import TaintLib
66
from ModuleValue m, string name, TaintedNode origin, TaintTrackingImplementation impl
77
where impl.moduleAttributeTainted(m, name, origin)
88

9-
select m.toString(), name, "Taint " + origin.getTaintKind(), origin.getContext(), origin.getLocation().toString()
9+
select m.toString(), name, origin.toString(), origin.getContext(), origin.getLocation().toString()

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
| test | carrier.py:4 | 18 | Attribute | test |
2+
| test | carrier.py:4 | 26 | Attribute() | test |
23
| test | test.py:12 | 13 | arg | test |
34
| test | test.py:46 | 13 | arg | test |
45
| test | test.py:49 | 13 | arg | test |

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

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
| carrier.py:4 | ParameterDefinition | carrier.py:4 | Taint explicit.carrier | arg |
22
| carrier.py:4 | ParameterDefinition | carrier.py:4 | Taint simple.test | arg |
3+
| carrier.py:10 | ParameterDefinition | carrier.py:10 | Taint .attr = simple.test | self |
4+
| carrier.py:13 | ParameterDefinition | carrier.py:13 | Taint .attr = simple.test | arg |
35
| carrier.py:13 | ParameterDefinition | carrier.py:13 | Taint explicit.carrier | arg |
6+
| carrier.py:17 | ImplicitCarrier() | carrier.py:17 | Taint .attr = simple.test | c |
47
| carrier.py:21 | TAINT_CARRIER_SOURCE | carrier.py:21 | Taint explicit.carrier | c |
8+
| carrier.py:25 | hub() | carrier.py:25 | Taint .attr = simple.test | c |
59
| carrier.py:29 | hub() | carrier.py:29 | Taint explicit.carrier | c |
10+
| carrier.py:33 | ImplicitCarrier() | carrier.py:33 | Taint .attr = explicit.carrier | c |
11+
| carrier.py:34 | Attribute | carrier.py:34 | Taint explicit.carrier | x |
612
| deep.py:2 | ParameterDefinition | deep.py:2 | Taint simple.test | arg |
713
| deep.py:5 | ParameterDefinition | deep.py:5 | Taint simple.test | arg |
814
| deep.py:8 | ParameterDefinition | deep.py:8 | Taint simple.test | arg |
@@ -37,16 +43,26 @@
3743
| sanitizer.py:31 | user_input() | sanitizer.py:31 | Taint SQL injection | x |
3844
| test.py:6 | SOURCE | test.py:6 | Taint simple.test | s |
3945
| test.py:12 | ParameterDefinition | test.py:12 | Taint simple.test | arg |
46+
| test.py:16 | source() | test.py:16 | Taint simple.test | t |
4047
| test.py:20 | SOURCE | test.py:20 | Taint simple.test | t |
48+
| test.py:24 | source() | test.py:24 | Taint simple.test | t |
4149
| test.py:31 | SOURCE | test.py:31 | Taint simple.test | t |
4250
| test.py:37 | SOURCE | test.py:37 | Taint simple.test | t |
51+
| test.py:46 | ParameterDefinition | test.py:46 | Taint simple.test | arg |
4352
| test.py:49 | ParameterDefinition | test.py:49 | Taint simple.test | arg |
53+
| test.py:54 | source2() | test.py:54 | Taint simple.test | t |
4454
| test.py:62 | SOURCE | test.py:62 | Taint simple.test | t |
4555
| test.py:67 | SOURCE | test.py:67 | Taint simple.test | t |
56+
| test.py:72 | ParameterDefinition | test.py:72 | Taint .x = simple.test | arg |
4657
| test.py:72 | ParameterDefinition | test.py:72 | Taint basic.custom | arg |
4758
| test.py:72 | ParameterDefinition | test.py:72 | Taint simple.test | arg |
4859
| test.py:76 | SOURCE | test.py:76 | Taint simple.test | t |
4960
| test.py:77 | hub() | test.py:77 | Taint simple.test | t |
61+
| test.py:85 | ImportExpr | test.py:85 | Taint .dangerous = simple.test | module |
62+
| test.py:88 | Attribute | test.py:88 | Taint simple.test | t |
63+
| test.py:100 | Attribute() | test.py:100 | Taint simple.test | t |
64+
| test.py:105 | ParameterDefinition | test.py:105 | Taint .x = simple.test | arg |
65+
| test.py:116 | hub() | test.py:116 | Taint .x = simple.test | t |
5066
| test.py:120 | CUSTOM_SOURCE | test.py:120 | Taint basic.custom | t |
5167
| test.py:121 | hub() | test.py:121 | Taint basic.custom | t |
5268
| test.py:126 | CUSTOM_SOURCE | test.py:126 | Taint basic.custom | t |
@@ -58,15 +74,15 @@
5874
| test.py:149 | TAINT_FROM_ARG() | test.py:149 | Taint basic.custom | t |
5975
| test.py:155 | ImportMember | test.py:155 | Taint simple.test | unsafe |
6076
| test.py:163 | SOURCE | test.py:163 | Taint simple.test | s |
61-
| test.py:168 | List | test.py:168 | Taint [simple.test] | l |
62-
| test.py:169 | Dict | test.py:169 | Taint {simple.test} | d |
63-
| test.py:174 | list() | test.py:174 | Taint [simple.test] | l2 |
64-
| test.py:175 | dict() | test.py:175 | Taint {simple.test} | d2 |
77+
| test.py:168 | List | test.py:168 | Taint sequence of simple.test | l |
78+
| test.py:169 | Dict | test.py:169 | Taint dict of simple.test | d |
79+
| test.py:174 | list() | test.py:174 | Taint sequence of simple.test | l2 |
80+
| test.py:175 | dict() | test.py:175 | Taint dict of simple.test | d2 |
6581
| test.py:178 | SOURCE | test.py:178 | Taint simple.test | t |
6682
| test.py:189 | FALSEY | test.py:189 | Taint falsey | t |
6783
| test.py:195 | SOURCE | test.py:195 | Taint simple.test | t |
6884
| test.py:202 | ITERABLE_SOURCE | test.py:202 | Taint iterable.simple | t |
6985
| test.py:203 | For | test.py:203 | Taint simple.test | i |
70-
| test.py:208 | List | test.py:208 | Taint [simple.test] | seq |
86+
| test.py:208 | List | test.py:208 | Taint sequence of simple.test | seq |
7187
| test.py:209 | For | test.py:209 | Taint simple.test | i |
7288
| test.py:213 | For | test.py:213 | Taint simple.test | x |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@ import TaintLib
66
from EssaNodeDefinition defn, TaintedNode n
77
where n.getNode().asVariable() = defn.getVariable()
88
select
9-
defn.getLocation().toString(), defn.getRepresentation(), n.getLocation().toString(), "Taint " + n.getTaintKind(), defn.getDefiningNode().getNode().toString()
9+
defn.getLocation().toString(), defn.getRepresentation(), n.getLocation().toString(), "Taint " + n.toString(), defn.getDefiningNode().getNode().toString()

0 commit comments

Comments
 (0)