Skip to content

Commit 30e2592

Browse files
RasmusWLRasmusWL
committed
Python: Propagate taint through parse_qs
1 parent 243dea7 commit 30e2592

File tree

4 files changed

+45
-12
lines changed

4 files changed

+45
-12
lines changed

‎python/ql/src/semmle/python/security/strings/External.qll

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ abstract class ExternalStringKind extends StringKind {
2222
urlsplit(fromnode, tonode) and result.(ExternalUrlSplitResult).getItem() = this
2323
or
2424
urlparse(fromnode, tonode) and result.(ExternalUrlParseResult).getItem() = this
25+
or
26+
parse_qs(fromnode, tonode) and result.(ExternalStringDictKind).getValue() = this
2527
}
2628
}
2729

@@ -181,6 +183,32 @@ private predicate urlparse(ControlFlowNode fromnode, CallNode tonode) {
181183
)
182184
}
183185

186+
private predicate parse_qs(ControlFlowNode fromnode, CallNode tonode) {
187+
// This could be implemented as `exists(FunctionValue` without the explicit six part,
188+
// but then our tests will need to import +100 modules, so for now this slightly
189+
// altered version gets to live on.
190+
exists(Value parse_qs |
191+
(
192+
parse_qs = Value::named("six.moves.urllib.parse.parse_qs")
193+
or
194+
// Python 2
195+
parse_qs = Value::named("urlparse.parse_qs")
196+
or
197+
// Python 2 deprecated version of `urlparse.parse_qs`
198+
parse_qs = Value::named("cgi.parse_qs")
199+
or
200+
// Python 3
201+
parse_qs = Value::named("urllib.parse.parse_qs")
202+
) and
203+
tonode = parse_qs.getACall() and
204+
(
205+
tonode.getArg(0) = fromnode
206+
or
207+
tonode.getArgByName("qs") = fromnode
208+
)
209+
)
210+
}
211+
184212
/** A kind of "taint", representing an open file-like object from an external source. */
185213
class ExternalFileObject extends TaintKind {
186214
ExternalFileObject() { this = "file[" + any(ExternalStringKind key) + "]" }

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
| Taint [externally controlled string] | test.py:67 | test.py:67:20:67:43 | urlsplit() | | --> | Taint [externally controlled string] | test.py:69 | test.py:69:10:69:21 | urlsplit_res | |
2-
| Taint [externally controlled string] | test.py:68 | test.py:68:20:68:43 | urlparse() | | --> | Taint [externally controlled string] | test.py:69 | test.py:69:24:69:35 | urlparse_res | |
1+
| Taint [externally controlled string] | test.py:67 | test.py:67:9:67:32 | urlsplit() | | --> | Taint [externally controlled string] | test.py:70 | test.py:70:10:70:10 | a | |
2+
| Taint [externally controlled string] | test.py:68 | test.py:68:9:68:32 | urlparse() | | --> | Taint [externally controlled string] | test.py:70 | test.py:70:13:70:13 | b | |
33
| Taint exception.info | test.py:44 | test.py:44:22:44:26 | taint | p1 = exception.info | --> | Taint exception.info | test.py:45 | test.py:45:17:45:21 | taint | p1 = exception.info |
44
| Taint exception.info | test.py:45 | test.py:45:17:45:21 | taint | p1 = exception.info | --> | Taint exception.info | test.py:45 | test.py:45:12:45:22 | func() | p1 = exception.info |
55
| Taint exception.info | test.py:45 | test.py:45:17:45:21 | taint | p1 = exception.info | --> | Taint exception.info | test.py:52 | test.py:52:19:52:21 | arg | p0 = exception.info |
@@ -58,10 +58,12 @@
5858
| Taint externally controlled string | test.py:57 | test.py:57:11:57:41 | cross_over() | | --> | Taint externally controlled string | test.py:58 | test.py:58:10:58:12 | res | |
5959
| Taint externally controlled string | test.py:57 | test.py:57:38:57:40 | ext | | --> | Taint externally controlled string | test.py:44 | test.py:44:22:44:26 | taint | p1 = externally controlled string |
6060
| Taint externally controlled string | test.py:57 | test.py:57:38:57:40 | ext | | --> | Taint externally controlled string | test.py:57 | test.py:57:11:57:41 | cross_over() | |
61-
| Taint externally controlled string | test.py:66 | test.py:66:22:66:35 | TAINTED_STRING | | --> | Taint externally controlled string | test.py:67 | test.py:67:29:67:42 | tainted_string | |
62-
| Taint externally controlled string | test.py:66 | test.py:66:22:66:35 | TAINTED_STRING | | --> | Taint externally controlled string | test.py:68 | test.py:68:29:68:42 | tainted_string | |
63-
| Taint externally controlled string | test.py:67 | test.py:67:29:67:42 | tainted_string | | --> | Taint [externally controlled string] | test.py:67 | test.py:67:20:67:43 | urlsplit() | |
64-
| Taint externally controlled string | test.py:68 | test.py:68:29:68:42 | tainted_string | | --> | Taint [externally controlled string] | test.py:68 | test.py:68:20:68:43 | urlparse() | |
61+
| Taint externally controlled string | test.py:66 | test.py:66:22:66:35 | TAINTED_STRING | | --> | Taint externally controlled string | test.py:67 | test.py:67:18:67:31 | tainted_string | |
62+
| Taint externally controlled string | test.py:66 | test.py:66:22:66:35 | TAINTED_STRING | | --> | Taint externally controlled string | test.py:68 | test.py:68:18:68:31 | tainted_string | |
63+
| Taint externally controlled string | test.py:66 | test.py:66:22:66:35 | TAINTED_STRING | | --> | Taint externally controlled string | test.py:69 | test.py:69:18:69:31 | tainted_string | |
64+
| Taint externally controlled string | test.py:67 | test.py:67:18:67:31 | tainted_string | | --> | Taint [externally controlled string] | test.py:67 | test.py:67:9:67:32 | urlsplit() | |
65+
| Taint externally controlled string | test.py:68 | test.py:68:18:68:31 | tainted_string | | --> | Taint [externally controlled string] | test.py:68 | test.py:68:9:68:32 | urlparse() | |
66+
| Taint externally controlled string | test.py:69 | test.py:69:18:69:31 | tainted_string | | --> | Taint {externally controlled string} | test.py:69 | test.py:69:9:69:32 | parse_qs() | |
6567
| Taint json[externally controlled string] | test.py:6 | test.py:6:20:6:45 | Attribute() | | --> | Taint json[externally controlled string] | test.py:7 | test.py:7:9:7:20 | tainted_json | |
6668
| Taint json[externally controlled string] | test.py:7 | test.py:7:9:7:20 | tainted_json | | --> | Taint externally controlled string | test.py:7 | test.py:7:9:7:25 | Subscript | |
6769
| Taint json[externally controlled string] | test.py:7 | test.py:7:9:7:20 | tainted_json | | --> | Taint json[externally controlled string] | test.py:7 | test.py:7:9:7:25 | Subscript | |
@@ -74,3 +76,4 @@
7476
| Taint json[externally controlled string] | test.py:9 | test.py:9:9:9:9 | b | | --> | Taint externally controlled string | test.py:9 | test.py:9:9:9:14 | Subscript | |
7577
| Taint json[externally controlled string] | test.py:9 | test.py:9:9:9:9 | b | | --> | Taint json[externally controlled string] | test.py:9 | test.py:9:9:9:14 | Subscript | |
7678
| Taint json[externally controlled string] | test.py:9 | test.py:9:9:9:14 | Subscript | | --> | Taint json[externally controlled string] | test.py:10 | test.py:10:16:10:16 | c | |
79+
| Taint {externally controlled string} | test.py:69 | test.py:69:9:69:32 | parse_qs() | | --> | Taint {externally controlled string} | test.py:70 | test.py:70:16:70:16 | c | |

‎python/ql/test/library-tests/taint/strings/TestTaint.expected

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,6 @@
2020
| test.py:42 | test_str2 | c | externally controlled string |
2121
| test.py:50 | test_exc_info | res | exception.info |
2222
| test.py:58 | test_untrusted | res | externally controlled string |
23-
| test.py:69 | test_urlsplit_urlparse | urlparse_res | [externally controlled string] |
24-
| test.py:69 | test_urlsplit_urlparse | urlsplit_res | [externally controlled string] |
23+
| test.py:70 | test_urlsplit_urlparse | a | [externally controlled string] |
24+
| test.py:70 | test_urlsplit_urlparse | b | [externally controlled string] |
25+
| test.py:70 | test_urlsplit_urlparse | c | {externally controlled string} |

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,11 @@ def test_untrusted():
6060
def exc_untrusted_call(arg):
6161
return arg
6262

63-
from six.moves.urllib.parse import urlsplit, urlparse
63+
from six.moves.urllib.parse import urlsplit, urlparse, parse_qs
6464

6565
def test_urlsplit_urlparse():
6666
tainted_string = TAINTED_STRING
67-
urlsplit_res = urlsplit(tainted_string)
68-
urlparse_res = urlparse(tainted_string)
69-
test(urlsplit_res, urlparse_res)
67+
a = urlsplit(tainted_string)
68+
b = urlparse(tainted_string)
69+
c = parse_qs(tainted_string)
70+
test(a, b, c)

0 commit comments

Comments
 (0)