Merge pull request github#3453 from RasmusWL/python-flask-routed-params

Approved by tausbn

Author: semmle-qlci

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

@@ -54,3 +54,35 @@ class FlaskRequestJson extends HttpRequestTaintSource {
 
     override string toString() { result = "flask.request.json" }
 }
+
+/**
+ * A parameter to a flask request handler, that can capture a part of the URL (as specified in
+ * the url-pattern of a route).
+ *
+ * For example, the `name` parameter in:
+ * ```
+ * @app.route('/hello/<name>')
+ * def hello(name):
+ * ```
+ */
+class FlaskRoutedParameter extends HttpRequestTaintSource {
+    FlaskRoutedParameter() {
+        exists(string name, Function func, StrConst url_pattern |
+            this.(ControlFlowNode).getNode() = func.getArgByName(name) and
+            flask_routing(url_pattern.getAFlowNode(), func) and
+            exists(string match |
+                match = url_pattern.getS().regexpFind(werkzeug_rule_re(), _, _) and
+                name = match.regexpCapture(werkzeug_rule_re(), 4)
+            )
+        )
+    }
+
+    override predicate isSourceOf(TaintKind kind) { kind instanceof ExternalStringKind }
+}
+
+private string werkzeug_rule_re() {
+    // since flask uses werkzeug internally, we are using its routing rules from
+    // https://github.com/pallets/werkzeug/blob/4dc8d6ab840d4b78cbd5789cef91b01e3bde01d5/src/werkzeug/routing.py#L138-L151
+    result =
+        "(?<static>[^<]*)<(?:(?<converter>[a-zA-Z_][a-zA-Z0-9_]*)(?:\\((?<args>.*?)\\))?\\:)?(?<variable>[a-zA-Z_][a-zA-Z0-9_]*)>"
+}

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

@@ -6,3 +6,11 @@
 | test.py:41:26:41:53 | flask.response.argument | externally controlled string |
 | test.py:46:12:46:62 | flask.routed.response | externally controlled string |
 | test.py:46:26:46:61 | flask.response.argument | externally controlled string |
+| test.py:50:12:50:48 | flask.routed.response | externally controlled string |
+| test.py:50:26:50:47 | flask.response.argument | externally controlled string |
+| test.py:54:12:54:53 | flask.routed.response | externally controlled string |
+| test.py:54:26:54:52 | flask.response.argument | externally controlled string |
+| test.py:60:12:60:62 | flask.routed.response | externally controlled string |
+| test.py:60:26:60:61 | flask.response.argument | externally controlled string |
+| test.py:64:12:64:58 | flask.routed.response | externally controlled string |
+| test.py:64:26:64:57 | flask.response.argument | externally controlled string |

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

@@ -3,3 +3,7 @@
 | test.py:35:16:35:27 | Attribute | {externally controlled string} |
 | test.py:40:18:40:29 | Attribute | {externally controlled string} |
 | test.py:45:18:45:29 | Attribute | {externally controlled string} |
+| test.py:49:11:49:14 | name | externally controlled string |
+| test.py:53:9:53:15 | subpath | externally controlled string |
+| test.py:59:24:59:26 | bar | externally controlled string |
+| test.py:63:13:63:21 | lang_code | externally controlled string |

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

@@ -1,6 +1,10 @@
-| / | Function hello |
+| / | Function hello_world |
+| /complex/<string(length=2):lang_code> | Function complex |
 | /dangerous | Function dangerous |
 | /dangerous-with-cfg-split | Function dangerous2 |
+| /foo/<path:subpath> | Function foo |
+| /hello/<name> | Function hello |
+| /multiple/bar/<bar> | Function multiple |
 | /safe | Function safe |
 | /the/ | Function get |
 | /unsafe | Function unsafe |

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

@@ -15,3 +15,19 @@
 | test.py:45 | Attribute() | externally controlled string |
 | test.py:46 | first_name | externally controlled string |
 | test.py:46 | make_response() | flask.Response |
+| test.py:49 | name | externally controlled string |
+| test.py:50 | BinaryExpr | externally controlled string |
+| test.py:50 | make_response() | flask.Response |
+| test.py:50 | name | externally controlled string |
+| test.py:53 | subpath | externally controlled string |
+| test.py:54 | BinaryExpr | externally controlled string |
+| test.py:54 | make_response() | flask.Response |
+| test.py:54 | subpath | externally controlled string |
+| test.py:59 | bar | externally controlled string |
+| test.py:60 | Attribute() | externally controlled string |
+| test.py:60 | bar | externally controlled string |
+| test.py:60 | make_response() | flask.Response |
+| test.py:63 | lang_code | externally controlled string |
+| test.py:64 | Attribute() | externally controlled string |
+| test.py:64 | lang_code | externally controlled string |
+| test.py:64 | make_response() | flask.Response |

python/ql/test/library-tests/web/flask/test.py

@@ -4,7 +4,7 @@
 app = Flask(__name__)
 
 @app.route("/")
-def hello():
+def hello_world():
     return "Hello World!"
 
 from flask.views import MethodView
@@ -44,3 +44,24 @@ def unsafe():
 def safe():
     first_name = request.args.get('name', '')
     return make_response("Your name is " + escape(first_name))
+
+@app.route('/hello/<name>')
+def hello(name):
+    return make_response("Your name is " + name)
+
+@app.route('/foo/<path:subpath>')
+def foo(subpath):
+    return make_response("The subpath is " + subpath)
+
+@app.route('/multiple/') # TODO: not recognized as route
+@app.route('/multiple/foo/<foo>') # TODO: not recognized as route
+@app.route('/multiple/bar/<bar>')
+def multiple(foo=None, bar=None):
+    return make_response("foo={!r} bar={!r}".format(foo, bar))
+
+@app.route('/complex/<string(length=2):lang_code>')
+def complex(lang_code):
+    return make_response("lang_code {}".format(lang_code))
+
+if __name__ == "__main__":
+    app.run(debug=True)