Skip to content

Commit 94ae2fe

Browse files
RasmusWLRasmusWL
committed
Python: Propagate taint through parse_qsl
1 parent 30e2592 commit 94ae2fe

File tree

4 files changed

+41
-8
lines changed

4 files changed

+41
-8
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
@@ -24,6 +24,8 @@ abstract class ExternalStringKind extends StringKind {
2424
urlparse(fromnode, tonode) and result.(ExternalUrlParseResult).getItem() = this
2525
or
2626
parse_qs(fromnode, tonode) and result.(ExternalStringDictKind).getValue() = this
27+
or
28+
parse_qsl(fromnode, tonode) and result.(SequenceKind).getItem().(SequenceKind).getItem() = this
2729
}
2830
}
2931

@@ -209,6 +211,32 @@ private predicate parse_qs(ControlFlowNode fromnode, CallNode tonode) {
209211
)
210212
}
211213

214+
private predicate parse_qsl(ControlFlowNode fromnode, CallNode tonode) {
215+
// This could be implemented as `exists(FunctionValue` without the explicit six part,
216+
// but then our tests will need to import +100 modules, so for now this slightly
217+
// altered version gets to live on.
218+
exists(Value parse_qsl |
219+
(
220+
parse_qsl = Value::named("six.moves.urllib.parse.parse_qsl")
221+
or
222+
// Python 2
223+
parse_qsl = Value::named("urlparse.parse_qsl")
224+
or
225+
// Python 2 deprecated version of `urlparse.parse_qsl`
226+
parse_qsl = Value::named("cgi.parse_qsl")
227+
or
228+
// Python 3
229+
parse_qsl = Value::named("urllib.parse.parse_qsl")
230+
) and
231+
tonode = parse_qsl.getACall() and
232+
(
233+
tonode.getArg(0) = fromnode
234+
or
235+
tonode.getArgByName("qs") = fromnode
236+
)
237+
)
238+
}
239+
212240
/** A kind of "taint", representing an open file-like object from an external source. */
213241
class ExternalFileObject extends TaintKind {
214242
ExternalFileObject() { this = "file[" + any(ExternalStringKind key) + "]" }

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
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 | |
1+
| Taint [[externally controlled string]] | test.py:70 | test.py:70:9:70:33 | parse_qsl() | | --> | Taint [[externally controlled string]] | test.py:71 | test.py:71:19:71:19 | d | |
2+
| Taint [externally controlled string] | test.py:67 | test.py:67:9:67:32 | urlsplit() | | --> | Taint [externally controlled string] | test.py:71 | test.py:71:10:71:10 | a | |
3+
| Taint [externally controlled string] | test.py:68 | test.py:68:9:68:32 | urlparse() | | --> | Taint [externally controlled string] | test.py:71 | test.py:71:13:71:13 | b | |
34
| 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 |
45
| 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 |
56
| 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 |
@@ -61,9 +62,11 @@
6162
| 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 | |
6263
| 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 | |
6364
| 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 | |
65+
| Taint externally controlled string | test.py:66 | test.py:66:22:66:35 | TAINTED_STRING | | --> | Taint externally controlled string | test.py:70 | test.py:70:19:70:32 | tainted_string | |
6466
| 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() | |
6567
| 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() | |
6668
| 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() | |
69+
| Taint externally controlled string | test.py:70 | test.py:70:19:70:32 | tainted_string | | --> | Taint [[externally controlled string]] | test.py:70 | test.py:70:9:70:33 | parse_qsl() | |
6770
| 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 | |
6871
| 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 | |
6972
| 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 | |
@@ -76,4 +79,4 @@
7679
| 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 | |
7780
| 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 | |
7881
| 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 | |
82+
| Taint {externally controlled string} | test.py:69 | test.py:69:9:69:32 | parse_qs() | | --> | Taint {externally controlled string} | test.py:71 | test.py:71:16:71:16 | c | |

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
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: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} |
23+
| test.py:71 | test_urlsplit_urlparse | a | [externally controlled string] |
24+
| test.py:71 | test_urlsplit_urlparse | b | [externally controlled string] |
25+
| test.py:71 | test_urlsplit_urlparse | c | {externally controlled string} |
26+
| test.py:71 | test_urlsplit_urlparse | d | [[externally controlled string]] |

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

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

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

6565
def test_urlsplit_urlparse():
6666
tainted_string = TAINTED_STRING
6767
a = urlsplit(tainted_string)
6868
b = urlparse(tainted_string)
6969
c = parse_qs(tainted_string)
70-
test(a, b, c)
70+
d = parse_qsl(tainted_string)
71+
test(a, b, c, d)

0 commit comments

Comments
 (0)