Python: correctly handle flask.make_response

Fixes https://github.com/Semmle/ql/issues/1572

Adjust mock so it's more aligned with what the flask code actually does. Tests
were passing before, even though we didn't handle the case in real code :\

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

@@ -1,5 +1,6 @@
 import python
 import semmle.python.web.Http
+import semmle.python.web.flask.Response
 
 /** The flask app class */
 ClassValue theFlaskClass() { result = Value::named("flask.Flask") }
@@ -92,7 +93,7 @@ private class AsView extends TaintSource {
 
 class FlaskCookieSet extends CookieSet, CallNode {
     FlaskCookieSet() {
-        this.getFunction().(AttrNode).getObject("set_cookie").pointsTo().getClass() = theFlaskReponseClass()
+        any(FlaskResponseTaintKind t).taints(this.getFunction().(AttrNode).getObject("set_cookie"))
     }
 
     override string toString() { result = CallNode.super.toString() }

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

@@ -23,7 +23,11 @@ class FlaskRoutedResponse extends HttpResponseTaintSink {
 class FlaskResponseArgument extends HttpResponseTaintSink {
     FlaskResponseArgument() {
         exists(CallNode call |
-            call.getFunction().pointsTo(theFlaskReponseClass()) and
+            (
+                call.getFunction().pointsTo(theFlaskReponseClass())
+                or
+                call.getFunction().pointsTo(Value::named("flask.make_response"))
+            ) and
             call.getArg(0) = this
         )
     }
@@ -32,3 +36,20 @@ class FlaskResponseArgument extends HttpResponseTaintSink {
 
     override string toString() { result = "flask.response.argument" }
 }
+
+class FlaskResponseTaintKind extends TaintKind {
+    FlaskResponseTaintKind() { this = "flask.Response" }
+}
+
+class FlaskResponseConfiguration extends TaintTracking::Configuration {
+    FlaskResponseConfiguration() { this = "Flask response configuration" }
+
+    override predicate isSource(DataFlow::Node node, TaintKind kind) {
+        kind instanceof FlaskResponseTaintKind and
+        (
+            node.asCfgNode().(CallNode).getFunction().pointsTo(theFlaskReponseClass())
+            or
+            node.asCfgNode().(CallNode).getFunction().pointsTo(Value::named("flask.make_response"))
+        )
+    }
+}

python/ql/test/query-tests/Security/CWE-079/ReflectedXss.expected

@@ -1,13 +1,9 @@
 edges
-| ../lib/flask/__init__.py:14:19:14:20 | externally controlled string | ../lib/flask/__init__.py:16:25:16:26 | externally controlled string |
-| ../lib/flask/__init__.py:14:19:14:20 | externally controlled string | ../lib/flask/__init__.py:16:25:16:26 | externally controlled string |
 | reflected_xss.py:7:18:7:29 | dict of externally controlled string | reflected_xss.py:7:18:7:45 | externally controlled string |
 | reflected_xss.py:7:18:7:29 | dict of externally controlled string | reflected_xss.py:7:18:7:45 | externally controlled string |
 | reflected_xss.py:7:18:7:45 | externally controlled string | reflected_xss.py:8:44:8:53 | externally controlled string |
 | reflected_xss.py:7:18:7:45 | externally controlled string | reflected_xss.py:8:44:8:53 | externally controlled string |
-| reflected_xss.py:8:26:8:53 | externally controlled string | ../lib/flask/__init__.py:14:19:14:20 | externally controlled string |
-| reflected_xss.py:8:26:8:53 | externally controlled string | ../lib/flask/__init__.py:14:19:14:20 | externally controlled string |
 | reflected_xss.py:8:44:8:53 | externally controlled string | reflected_xss.py:8:26:8:53 | externally controlled string |
 | reflected_xss.py:8:44:8:53 | externally controlled string | reflected_xss.py:8:26:8:53 | externally controlled string |
 #select
-| ../lib/flask/__init__.py:16:25:16:26 | rv | reflected_xss.py:7:18:7:29 | dict of externally controlled string | ../lib/flask/__init__.py:16:25:16:26 | externally controlled string | Cross-site scripting vulnerability due to $@. | reflected_xss.py:7:18:7:29 | Attribute | user-provided value |
+| reflected_xss.py:8:26:8:53 | BinaryExpr | reflected_xss.py:7:18:7:29 | dict of externally controlled string | reflected_xss.py:8:26:8:53 | externally controlled string | Cross-site scripting vulnerability due to $@. | reflected_xss.py:7:18:7:29 | Attribute | user-provided value |

python/ql/test/query-tests/Security/CWE-079/reflected_xss.py

@@ -11,8 +11,3 @@ def unsafe():
 def safe():
     first_name = request.args.get('name', '')
     return make_response("Your name is " + escape(first_name))
-
-urlpatterns = [
-    url(r'^r1$', response_unsafe, name='response-unsafe'),
-    url(r'^r2$', response_safe, name='response-safe')
-]

python/ql/test/query-tests/Security/lib/flask/__init__.py

@@ -1,9 +1,17 @@
-
-
-class Flask(object): 
-    def run(self, *args, **kwargs): pass
-
 from .globals import request
+from .globals import current_app
+
+class Flask(object):
+    # Only some methods mocked, signature copied from
+    # https://flask.palletsprojects.com/en/1.1.x/api/#flask.Flask
+    def run(host=None, port=None, debug=None, load_dotenv=True, **options):
+        pass
+
+    def make_response(rv):
+        pass
+
+    def add_url_rule(rule, endpoint=None, view_func=None, provide_automatic_options=None, **options):
+        pass
 
 class Response(object):
     pass
@@ -12,12 +20,11 @@ def redirect(location, code=302, Response=None):
     pass
 
 def make_response(rv):
-    if isinstance(rv, str):
-        return Response(rv)
-    elif isinstance(rv, Response):
-        return rv
-    else:
-        pass
+    if not args:
+        return current_app.response_class()
+    if len(args) == 1:
+        args = args[0]
+    return current_app.make_response(args)
 
 def escape(txt):
     return Markup.escape(txt)

python/ql/test/query-tests/Security/lib/flask/globals.py

@@ -3,3 +3,4 @@ class LocalProxy(object):
     pass
 
 request = LocalProxy()
+current_app = LocalProxy()

python/ql/test/query-tests/Security/lib/flask/views.py

@@ -1,11 +1,6 @@
-
-
-
 class View(object):
     pass
 
 
 class MethodView(object):
     pass
-
-