Skip to content

Commit 8150c78

Browse files
committed
Python: In flask, taint routed prameters for variable rules
Fixes github/codeql-python-team#79
1 parent 7d5e35a commit 8150c78

File tree

3 files changed

+48
-0
lines changed

3 files changed

+48
-0
lines changed

python/ql/src/semmle/python/web/flask/Request.qll

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,35 @@ class FlaskRequestJson extends HttpRequestTaintSource {
5454

5555
override string toString() { result = "flask.request.json" }
5656
}
57+
58+
/**
59+
* A parameter to a flask request handler, that can capture a part of the URL (as specified in
60+
* the url-pattern of a route).
61+
*
62+
* For example, the `name` parameter in:
63+
* ```
64+
* @app.route('/hello/<name>')
65+
* def hello(name):
66+
* ```
67+
*/
68+
class FlaskRoutedParameter extends HttpRequestTaintSource {
69+
FlaskRoutedParameter() {
70+
exists(string name, Function func, StrConst url_pattern |
71+
this.(ControlFlowNode).getNode() = func.getArgByName(name) and
72+
flask_routing(url_pattern.getAFlowNode(), func) and
73+
exists(string match |
74+
match = url_pattern.getS().regexpFind(werkzeug_rule_re(), _, _) and
75+
name = match.regexpCapture(werkzeug_rule_re(), 4)
76+
)
77+
)
78+
}
79+
80+
override predicate isSourceOf(TaintKind kind) { kind instanceof ExternalStringKind }
81+
}
82+
83+
private string werkzeug_rule_re() {
84+
// since flask uses werkzeug internally, we are using it's routing rules from
85+
// https://github.com/pallets/werkzeug/blob/4dc8d6ab840d4b78cbd5789cef91b01e3bde01d5/src/werkzeug/routing.py#L138-L151
86+
result =
87+
"(?<static>[^<]*)<(?:(?<converter>[a-zA-Z_][a-zA-Z0-9_]*)(?:\\((?<args>.*?)\\))?\\:)?(?<variable>[a-zA-Z_][a-zA-Z0-9_]*)>"
88+
}

python/ql/test/library-tests/web/flask/HttpSources.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,7 @@
33
| test.py:35:16:35:27 | Attribute | {externally controlled string} |
44
| test.py:40:18:40:29 | Attribute | {externally controlled string} |
55
| test.py:45:18:45:29 | Attribute | {externally controlled string} |
6+
| test.py:49:11:49:14 | name | externally controlled string |
7+
| test.py:53:9:53:15 | subpath | externally controlled string |
8+
| test.py:59:24:59:26 | bar | externally controlled string |
9+
| test.py:63:13:63:21 | lang_code | externally controlled string |

python/ql/test/library-tests/web/flask/Taint.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,19 @@
1515
| test.py:45 | Attribute() | externally controlled string |
1616
| test.py:46 | first_name | externally controlled string |
1717
| test.py:46 | make_response() | flask.Response |
18+
| test.py:49 | name | externally controlled string |
19+
| test.py:50 | BinaryExpr | externally controlled string |
1820
| test.py:50 | make_response() | flask.Response |
21+
| test.py:50 | name | externally controlled string |
22+
| test.py:53 | subpath | externally controlled string |
23+
| test.py:54 | BinaryExpr | externally controlled string |
1924
| test.py:54 | make_response() | flask.Response |
25+
| test.py:54 | subpath | externally controlled string |
26+
| test.py:59 | bar | externally controlled string |
27+
| test.py:60 | Attribute() | externally controlled string |
28+
| test.py:60 | bar | externally controlled string |
2029
| test.py:60 | make_response() | flask.Response |
30+
| test.py:63 | lang_code | externally controlled string |
31+
| test.py:64 | Attribute() | externally controlled string |
32+
| test.py:64 | lang_code | externally controlled string |
2133
| test.py:64 | make_response() | flask.Response |

0 commit comments

Comments
 (0)