Skip to content

Commit f5ccd3c

Browse files
taus-semmletaus-semmle
authored
Merge pull request github#1210 from markshannon/python-dataflow-config
Python: Make DataFlow::Configuration act more like other languages
2 parents fa8b771 + df2000e commit f5ccd3c

File tree

8 files changed

+292
-22
lines changed

8 files changed

+292
-22
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import semmle.python.security.TaintTracking

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

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -326,8 +326,6 @@ abstract class Sanitizer extends string {
326326
private predicate valid_sanitizer(Sanitizer sanitizer) {
327327
not exists(TaintTracking::Configuration c)
328328
or
329-
exists(DataFlow::Configuration c | c.isSanitizer(sanitizer))
330-
or
331329
exists(TaintTracking::Configuration c | c.isSanitizer(sanitizer))
332330
}
333331

@@ -600,7 +598,7 @@ private newtype TTaintedNode =
600598
exists(DataFlow::Configuration config, TaintKind kind |
601599
taint = TaintFlowImplementation::TTrackedTaint(kind) and
602600
config.isSource(n) and context.getDepth() = 0 and
603-
kind instanceof GenericFlowType
601+
kind instanceof DataFlowType
604602
)
605603
or
606604
TaintFlowImplementation::step(_, taint, context, n) and
@@ -864,8 +862,6 @@ library module TaintFlowImplementation {
864862
(
865863
not exists(TaintTracking::Configuration c)
866864
or
867-
exists(DataFlow::Configuration c | c.isExtension(fromnodenode))
868-
or
869865
exists(TaintTracking::Configuration c | c.isExtension(fromnodenode))
870866
)
871867
|
@@ -1090,8 +1086,6 @@ library module TaintFlowImplementation {
10901086
(
10911087
not exists(TaintTracking::Configuration c)
10921088
or
1093-
exists(DataFlow::Configuration c | c.isExtension(originnode))
1094-
or
10951089
exists(TaintTracking::Configuration c | c.isExtension(originnode))
10961090
) and
10971091
originnode.getASuccessorVariable() = var and
@@ -1537,16 +1531,12 @@ class CallContext extends TCallContext {
15371531
*/
15381532
module DataFlow {
15391533

1540-
class FlowType = TaintKind;
1541-
15421534
/** Generic taint kind, source and sink classes for convenience and
15431535
* compatibility with other language libraries
15441536
*/
15451537

15461538
class Node = ControlFlowNode;
15471539

1548-
class PathNode = TaintedNode;
1549-
15501540
class Extension = DataFlowExtension::DataFlowNode;
15511541

15521542
abstract class Configuration extends string {
@@ -1558,19 +1548,14 @@ module DataFlow {
15581548

15591549
abstract predicate isSink(Node sink);
15601550

1561-
predicate isSanitizer(Sanitizer sanitizer) { none() }
1562-
1563-
predicate isExtension(Extension extension) { none() }
1564-
1565-
predicate hasFlowPath(PathNode source, PathNode sink) {
1551+
private predicate hasFlowPath(TaintedNode source, TaintedNode sink) {
15661552
this.isSource(source.getNode()) and
15671553
this.isSink(sink.getNode()) and
1568-
source.getTaintKind() instanceof GenericFlowType and
1569-
sink.getTaintKind() instanceof GenericFlowType
1554+
source.getASuccessor*() = sink
15701555
}
15711556

15721557
predicate hasFlow(Node source, Node sink) {
1573-
exists(PathNode psource, PathNode psink |
1558+
exists(TaintedNode psource, TaintedNode psink |
15741559
psource.getNode() = source and
15751560
psink.getNode() = sink and
15761561
this.isSource(source) and
@@ -1583,10 +1568,10 @@ module DataFlow {
15831568

15841569
}
15851570

1586-
private class GenericFlowType extends DataFlow::FlowType {
1571+
private class DataFlowType extends TaintKind {
15871572

1588-
GenericFlowType() {
1589-
this = "Generic taint kind" and
1573+
DataFlowType() {
1574+
this = "Data flow" and
15901575
exists(DataFlow::Configuration c)
15911576
}
15921577

‎python/ql/test/library-tests/taint/dataflow/Config.qll

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import python
2+
import semmle.python.dataflow.DataFlow
3+
4+
class TestConfiguration extends DataFlow::Configuration {
5+
6+
TestConfiguration() { this = "Test configuration" }
7+
8+
override predicate isSource(ControlFlowNode source) { source.(NameNode).getId() = "SOURCE" }
9+
10+
override predicate isSink(ControlFlowNode sink) {
11+
exists(CallNode call |
12+
call.getFunction().(NameNode).getId() = "SINK" and
13+
sink = call.getAnArg()
14+
)
15+
}
16+
17+
}

‎python/ql/test/library-tests/taint/dataflow/Dataflow.expected

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
| test.py:3:10:3:15 | ControlFlowNode for SOURCE | test.py:3:10:3:15 | ControlFlowNode for SOURCE |
2+
| test.py:6:9:6:14 | ControlFlowNode for SOURCE | test.py:7:10:7:10 | ControlFlowNode for s |
3+
| test.py:10:12:10:17 | ControlFlowNode for SOURCE | test.py:13:10:13:12 | ControlFlowNode for arg |
4+
| test.py:10:12:10:17 | ControlFlowNode for SOURCE | test.py:17:10:17:10 | ControlFlowNode for t |
5+
| test.py:20:9:20:14 | ControlFlowNode for SOURCE | test.py:13:10:13:12 | ControlFlowNode for arg |
6+
| test.py:37:13:37:18 | ControlFlowNode for SOURCE | test.py:41:14:41:14 | ControlFlowNode for t |
7+
| test.py:62:13:62:18 | ControlFlowNode for SOURCE | test.py:13:10:13:12 | ControlFlowNode for arg |
8+
| test.py:67:13:67:18 | ControlFlowNode for SOURCE | test.py:13:10:13:12 | ControlFlowNode for arg |
9+
| test.py:76:9:76:14 | ControlFlowNode for SOURCE | test.py:78:10:78:10 | ControlFlowNode for t |
10+
| test.py:108:13:108:18 | ControlFlowNode for SOURCE | test.py:112:14:112:14 | ControlFlowNode for t |
11+
| test.py:139:10:139:15 | ControlFlowNode for SOURCE | test.py:140:14:140:14 | ControlFlowNode for t |
12+
| test.py:143:9:143:14 | ControlFlowNode for SOURCE | test.py:145:10:145:10 | ControlFlowNode for s |
13+
| test.py:148:10:148:15 | ControlFlowNode for SOURCE | test.py:152:10:152:13 | ControlFlowNode for Subscript |
14+
| test.py:149:18:149:23 | ControlFlowNode for SOURCE | test.py:153:10:153:17 | ControlFlowNode for Subscript |
15+
| test.py:158:9:158:14 | ControlFlowNode for SOURCE | test.py:160:14:160:14 | ControlFlowNode for t |
16+
| test.py:158:9:158:14 | ControlFlowNode for SOURCE | test.py:166:14:166:14 | ControlFlowNode for t |

‎python/ql/test/library-tests/taint/dataflow/Dataflow.ql

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
2+
import python
3+
import Config
4+
5+
from TestConfiguration config, ControlFlowNode src, ControlFlowNode sink
6+
where config.hasFlow(src, sink)
7+
select src, sink

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
| Taint Data flow | test.py:3 | SOURCE | |
2+
| Taint Data flow | test.py:6 | SOURCE | |
3+
| Taint Data flow | test.py:7 | s | |
4+
| Taint Data flow | test.py:10 | SOURCE | |
5+
| Taint Data flow | test.py:12 | arg | test.py:21 |
6+
| Taint Data flow | test.py:12 | arg | test.py:25 |
7+
| Taint Data flow | test.py:12 | arg | test.py:47 from test.py:55 |
8+
| Taint Data flow | test.py:12 | arg | test.py:51 from test.py:63 |
9+
| Taint Data flow | test.py:12 | arg | test.py:51 from test.py:70 |
10+
| Taint Data flow | test.py:13 | arg | test.py:21 |
11+
| Taint Data flow | test.py:13 | arg | test.py:25 |
12+
| Taint Data flow | test.py:13 | arg | test.py:47 from test.py:55 |
13+
| Taint Data flow | test.py:13 | arg | test.py:51 from test.py:63 |
14+
| Taint Data flow | test.py:13 | arg | test.py:51 from test.py:70 |
15+
| Taint Data flow | test.py:16 | source() | |
16+
| Taint Data flow | test.py:17 | t | |
17+
| Taint Data flow | test.py:20 | SOURCE | |
18+
| Taint Data flow | test.py:21 | t | |
19+
| Taint Data flow | test.py:24 | source() | |
20+
| Taint Data flow | test.py:25 | t | |
21+
| Taint Data flow | test.py:31 | SOURCE | |
22+
| Taint Data flow | test.py:37 | SOURCE | |
23+
| Taint Data flow | test.py:41 | t | |
24+
| Taint Data flow | test.py:44 | source() | |
25+
| Taint Data flow | test.py:46 | arg | test.py:55 |
26+
| Taint Data flow | test.py:47 | arg | test.py:55 |
27+
| Taint Data flow | test.py:49 | arg | test.py:63 |
28+
| Taint Data flow | test.py:49 | arg | test.py:70 |
29+
| Taint Data flow | test.py:51 | arg | test.py:63 |
30+
| Taint Data flow | test.py:51 | arg | test.py:70 |
31+
| Taint Data flow | test.py:54 | source2() | |
32+
| Taint Data flow | test.py:55 | t | |
33+
| Taint Data flow | test.py:62 | SOURCE | |
34+
| Taint Data flow | test.py:63 | t | |
35+
| Taint Data flow | test.py:67 | SOURCE | |
36+
| Taint Data flow | test.py:70 | t | |
37+
| Taint Data flow | test.py:72 | arg | test.py:77 |
38+
| Taint Data flow | test.py:73 | arg | test.py:77 |
39+
| Taint Data flow | test.py:76 | SOURCE | |
40+
| Taint Data flow | test.py:77 | hub() | |
41+
| Taint Data flow | test.py:77 | t | |
42+
| Taint Data flow | test.py:78 | t | |
43+
| Taint Data flow | test.py:108 | SOURCE | |
44+
| Taint Data flow | test.py:112 | t | |
45+
| Taint Data flow | test.py:118 | SOURCE | |
46+
| Taint Data flow | test.py:120 | t | |
47+
| Taint Data flow | test.py:128 | SOURCE | |
48+
| Taint Data flow | test.py:129 | t | |
49+
| Taint Data flow | test.py:139 | SOURCE | |
50+
| Taint Data flow | test.py:140 | t | |
51+
| Taint Data flow | test.py:143 | SOURCE | |
52+
| Taint Data flow | test.py:144 | s | |
53+
| Taint Data flow | test.py:145 | s | |
54+
| Taint Data flow | test.py:148 | SOURCE | |
55+
| Taint Data flow | test.py:149 | SOURCE | |
56+
| Taint Data flow | test.py:152 | Subscript | |
57+
| Taint Data flow | test.py:153 | Subscript | |
58+
| Taint Data flow | test.py:158 | SOURCE | |
59+
| Taint Data flow | test.py:159 | t | |
60+
| Taint Data flow | test.py:160 | t | |
61+
| Taint Data flow | test.py:163 | t | |
62+
| Taint Data flow | test.py:166 | t | |
63+
| Taint [Data flow] | test.py:148 | List | |
64+
| Taint [Data flow] | test.py:150 | l | |
65+
| Taint [Data flow] | test.py:152 | x | |
66+
| Taint [Data flow] | test.py:154 | l | |
67+
| Taint [Data flow] | test.py:154 | list() | |
68+
| Taint {Data flow} | test.py:149 | Dict | |
69+
| Taint {Data flow} | test.py:151 | d | |
70+
| Taint {Data flow} | test.py:153 | y | |
71+
| Taint {Data flow} | test.py:155 | d | |
72+
| Taint {Data flow} | test.py:155 | dict() | |

‎python/ql/test/library-tests/taint/dataflow/TestNode.ql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import python
2+
import Config
3+
4+
from TaintedNode n
5+
select n.getTrackedValue(), n.getLocation().toString(), n.getNode().getNode().toString(), n.getContext()

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

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
2+
def test1():
3+
SINK(SOURCE)
4+
5+
def test2():
6+
s = SOURCE
7+
SINK(s)
8+
9+
def source():
10+
return SOURCE
11+
12+
def sink(arg):
13+
SINK(arg)
14+
15+
def test3():
16+
t = source()
17+
SINK(t)
18+
19+
def test4():
20+
t = SOURCE
21+
sink(t)
22+
23+
def test5():
24+
t = source()
25+
sink(t)
26+
27+
def test6(cond):
28+
if cond:
29+
t = "Safe"
30+
else:
31+
t = SOURCE
32+
if cond:
33+
SINK(t)
34+
35+
def test7(cond):
36+
if cond:
37+
t = SOURCE
38+
else:
39+
t = "Safe"
40+
if cond:
41+
SINK(t)
42+
43+
def source2(arg):
44+
return source(arg)
45+
46+
def sink2(arg):
47+
sink(arg)
48+
49+
def sink3(cond, arg):
50+
if cond:
51+
sink(arg)
52+
53+
def test8(cond):
54+
t = source2()
55+
sink2(t)
56+
57+
#False positive
58+
def test9(cond):
59+
if cond:
60+
t = "Safe"
61+
else:
62+
t = SOURCE
63+
sink3(cond, t)
64+
65+
def test10(cond):
66+
if cond:
67+
t = SOURCE
68+
else:
69+
t = "Safe"
70+
sink3(cond, t)
71+
72+
def hub(arg):
73+
return arg
74+
75+
def test11():
76+
t = SOURCE
77+
t = hub(t)
78+
SINK(t)
79+
80+
def test12():
81+
t = "safe"
82+
t = hub(t)
83+
SINK(t)
84+
85+
import module
86+
87+
def test13():
88+
t = module.dangerous
89+
SINK(t)
90+
91+
def test14():
92+
t = module.safe
93+
SINK(t)
94+
95+
def test15():
96+
t = module.safe2
97+
SINK(t)
98+
99+
def test16():
100+
t = module.dangerous_func()
101+
SINK(t)
102+
103+
104+
def test20(cond):
105+
if cond:
106+
t = CUSTOM_SOURCE
107+
else:
108+
t = SOURCE
109+
if cond:
110+
CUSTOM_SINK(t)
111+
else:
112+
SINK(t)
113+
114+
def test21(cond):
115+
if cond:
116+
t = CUSTOM_SOURCE
117+
else:
118+
t = SOURCE
119+
if not cond:
120+
CUSTOM_SINK(t)
121+
else:
122+
SINK(t)
123+
124+
def test22(cond):
125+
if cond:
126+
t = CUSTOM_SOURCE
127+
else:
128+
t = SOURCE
129+
t = TAINT_FROM_ARG(t)
130+
if cond:
131+
CUSTOM_SINK(t)
132+
else:
133+
SINK(t)
134+
135+
from module import dangerous as unsafe
136+
SINK(unsafe)
137+
138+
def test23():
139+
with SOURCE as t:
140+
SINK(t)
141+
142+
def test24():
143+
s = SOURCE
144+
SANITIZE(s)
145+
SINK(s)
146+
147+
def test_update_extend(x, y):
148+
l = [SOURCE]
149+
d = {"key" : SOURCE}
150+
x.extend(l)
151+
y.update(d)
152+
SINK(x[0])
153+
SINK(y["key"])
154+
l2 = list(l)
155+
d2 = dict(d)
156+
157+
def test_truth():
158+
t = SOURCE
159+
if t:
160+
SINK(t)
161+
else:
162+
SINK(t)
163+
if not t:
164+
SINK(t)
165+
else:
166+
SINK(t)
167+

0 commit comments

Comments
 (0)