Merge pull request github#4944 from RasmusWL/flask-class-based-handlers

Python: Add modeling of Flask class based (HTTP) request handlers

Author: yoff

python/ql/src/semmle/python/frameworks/Flask.qll

@@ -34,7 +34,7 @@ private module FlaskModel {
    * WARNING: Only holds for a few predefined attributes.
    */
   private DataFlow::Node flask_attr(DataFlow::TypeTracker t, string attr_name) {
-    attr_name in ["request", "make_response", "Response"] and
+    attr_name in ["request", "make_response", "Response", "views"] and
     (
       t.start() and
       result = DataFlow::importNode("flask" + "." + attr_name)
@@ -187,6 +187,97 @@ private module FlaskModel {
        */
       DataFlow::Node response_class() { result = response_class(DataFlow::TypeTracker::end()) }
     }
+
+    // -------------------------------------------------------------------------
+    // flask.views
+    // -------------------------------------------------------------------------
+    /** Gets a reference to the `flask.views` module. */
+    DataFlow::Node views() { result = flask_attr("views") }
+
+    /** Provides models for the `flask.views` module */
+    module views {
+      /**
+       * Gets a reference to the attribute `attr_name` of the `flask.views` module.
+       * WARNING: Only holds for a few predefined attributes.
+       */
+      private DataFlow::Node views_attr(DataFlow::TypeTracker t, string attr_name) {
+        attr_name in ["View", "MethodView"] and
+        (
+          t.start() and
+          result = DataFlow::importNode("flask.views" + "." + attr_name)
+          or
+          t.startInAttr(attr_name) and
+          result = views()
+        )
+        or
+        // Due to bad performance when using normal setup with `views_attr(t2, attr_name).track(t2, t)`
+        // we have inlined that code and forced a join
+        exists(DataFlow::TypeTracker t2 |
+          exists(DataFlow::StepSummary summary |
+            views_attr_first_join(t2, attr_name, result, summary) and
+            t = t2.append(summary)
+          )
+        )
+      }
+
+      pragma[nomagic]
+      private predicate views_attr_first_join(
+        DataFlow::TypeTracker t2, string attr_name, DataFlow::Node res,
+        DataFlow::StepSummary summary
+      ) {
+        DataFlow::StepSummary::step(views_attr(t2, attr_name), res, summary)
+      }
+
+      /**
+       * Gets a reference to the attribute `attr_name` of the `flask.views` module.
+       * WARNING: Only holds for a few predefined attributes.
+       */
+      private DataFlow::Node views_attr(string attr_name) {
+        result = views_attr(DataFlow::TypeTracker::end(), attr_name)
+      }
+
+      /**
+       * Provides models for the `flask.views.View` class and subclasses.
+       *
+       * See https://flask.palletsprojects.com/en/1.1.x/views/#basic-principle.
+       */
+      module View {
+        /** Gets a reference to the `flask.views.View` class or any subclass. */
+        private DataFlow::Node subclassRef(DataFlow::TypeTracker t) {
+          t.start() and
+          result = views_attr(["View", "MethodView"])
+          or
+          // subclasses in project code
+          result.asExpr().(ClassExpr).getABase() = subclassRef(t.continue()).asExpr()
+          or
+          exists(DataFlow::TypeTracker t2 | result = subclassRef(t2).track(t2, t))
+        }
+
+        /** Gets a reference to the `flask.views.View` class or any subclass. */
+        DataFlow::Node subclassRef() { result = subclassRef(DataFlow::TypeTracker::end()) }
+      }
+
+      /**
+       * Provides models for the `flask.views.MethodView` class and subclasses.
+       *
+       * See https://flask.palletsprojects.com/en/1.1.x/views/#method-based-dispatching.
+       */
+      module MethodView {
+        /** Gets a reference to the `flask.views.View` class or any subclass. */
+        private DataFlow::Node subclassRef(DataFlow::TypeTracker t) {
+          t.start() and
+          result = views_attr("MethodView")
+          or
+          // subclasses in project code
+          result.asExpr().(ClassExpr).getABase() = subclassRef(t.continue()).asExpr()
+          or
+          exists(DataFlow::TypeTracker t2 | result = subclassRef(t2).track(t2, t))
+        }
+
+        /** Gets a reference to the `flask.views.View` class or any subclass. */
+        DataFlow::Node subclassRef() { result = subclassRef(DataFlow::TypeTracker::end()) }
+      }
+    }
   }
 
   /**
@@ -261,6 +352,65 @@ private module FlaskModel {
   // ---------------------------------------------------------------------------
   // routing modeling
   // ---------------------------------------------------------------------------
+  /** A flask View class defined in project code. */
+  class FlaskViewClassDef extends Class {
+    FlaskViewClassDef() { this.getABase() = flask::views::View::subclassRef().asExpr() }
+
+    /** Gets a function that could handle incoming requests, if any. */
+    Function getARequestHandler() {
+      // TODO: This doesn't handle attribute assignment. Should be OK, but analysis is not as complete as with
+      // points-to and `.lookup`, which would handle `post = my_post_handler` inside class def
+      result = this.getAMethod() and
+      result.getName() = "dispatch_request"
+    }
+
+    /** Gets a reference to this class. */
+    private DataFlow::Node getARef(DataFlow::TypeTracker t) {
+      t.start() and
+      result.asExpr().(ClassExpr) = this.getParent()
+      or
+      exists(DataFlow::TypeTracker t2 | result = this.getARef(t2).track(t2, t))
+    }
+
+    /** Gets a reference to this class. */
+    DataFlow::Node getARef() { result = this.getARef(DataFlow::TypeTracker::end()) }
+
+    /** Gets a reference to the `as_view` classmethod of this class. */
+    private DataFlow::Node asViewRef(DataFlow::TypeTracker t) {
+      t.startInAttr("as_view") and
+      result = this.getARef()
+      or
+      exists(DataFlow::TypeTracker t2 | result = this.asViewRef(t2).track(t2, t))
+    }
+
+    /** Gets a reference to the `as_view` classmethod of this class. */
+    DataFlow::Node asViewRef() { result = this.asViewRef(DataFlow::TypeTracker::end()) }
+
+    /** Gets a reference to the result of calling the `as_view` classmethod of this class. */
+    private DataFlow::Node asViewResult(DataFlow::TypeTracker t) {
+      t.start() and
+      result.asCfgNode().(CallNode).getFunction() = this.asViewRef().asCfgNode()
+      or
+      exists(DataFlow::TypeTracker t2 | result = asViewResult(t2).track(t2, t))
+    }
+
+    /** Gets a reference to the result of calling the `as_view` classmethod of this class. */
+    DataFlow::Node asViewResult() { result = asViewResult(DataFlow::TypeTracker::end()) }
+  }
+
+  class FlaskMethodViewClassDef extends FlaskViewClassDef {
+    FlaskMethodViewClassDef() { this.getABase() = flask::views::MethodView::subclassRef().asExpr() }
+
+    override Function getARequestHandler() {
+      result = super.getARequestHandler()
+      or
+      // TODO: This doesn't handle attribute assignment. Should be OK, but analysis is not as complete as with
+      // points-to and `.lookup`, which would handle `post = my_post_handler` inside class def
+      result = this.getAMethod() and
+      result.getName() = HTTP::httpVerbLower()
+    }
+  }
+
   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
@@ -318,12 +468,36 @@ private module FlaskModel {
       result.asCfgNode() in [node.getArg(0), node.getArgByName("rule")]
     }
 
+    DataFlow::Node getViewArg() {
+      result.asCfgNode() in [node.getArg(2), node.getArgByName("view_func")]
+    }
+
     override Function getARequestHandler() {
-      exists(DataFlow::Node view_func_arg, DataFlow::LocalSourceNode func_src |
-        view_func_arg.asCfgNode() in [node.getArg(2), node.getArgByName("view_func")] and
-        func_src.flowsTo(view_func_arg) and
+      exists(DataFlow::LocalSourceNode func_src |
+        func_src.flowsTo(getViewArg()) and
         func_src.asExpr().(CallableExpr) = result.getDefinition()
       )
+      or
+      exists(FlaskViewClassDef vc |
+        getViewArg() = vc.asViewResult() and
+        result = vc.getARequestHandler()
+      )
+    }
+  }
+
+  /** A request handler defined in a django view class, that has no known route. */
+  private class FlaskViewClassHandlerWithoutKnownRoute extends HTTP::Server::RequestHandler::Range {
+    FlaskViewClassHandlerWithoutKnownRoute() {
+      exists(FlaskViewClassDef vc | vc.getARequestHandler() = this) and
+      not exists(FlaskRouteSetup setup | setup.getARequestHandler() = this)
+    }
+
+    override Parameter getARoutedParameter() {
+      // Since we don't know the URL pattern, we simply mark all parameters as a routed
+      // parameter. This should give us more RemoteFlowSources but could also lead to
+      // more FPs. If this turns out to be the wrong tradeoff, we can always change our mind.
+      result in [this.getArg(_), this.getArgByName(_)] and
+      not result = this.getArg(0)
     }
   }
 

python/ql/test/experimental/library-tests/frameworks/flask/old_test.py

@@ -11,7 +11,7 @@ def hello_world():  # $requestHandler
 
 class MyView(MethodView):
 
-    def get(self, user_id): # $ MISSING: requestHandler
+    def get(self, user_id): # $ requestHandler
         if user_id is None:
             # return a list of users
             pass

python/ql/test/experimental/library-tests/frameworks/flask/routing_test.py

@@ -21,10 +21,76 @@ def later_set():  # $ MISSING: requestHandler
 app.add_url_rule('/later-set', 'later_set', view_func=None)  # $routeSetup="/later-set"
 app.view_functions['later_set'] = later_set
 
+# We don't want to execute this at runtime (since program will crash). Just using
+# `False` makes our analysis skip it, so here's a workaround :D
+if __file__ == "False":
+    @app.route(UNKNOWN_ROUTE) # $routeSetup
+    def unkown_route(foo, bar):  # $requestHandler routedParameter=foo routedParameter=bar
+        return make_response("unkown_route")  # $HttpResponse
 
-@app.route(UNKNOWN_ROUTE) # $routeSetup
-def unkown_route(foo, bar):  # $requestHandler routedParameter=foo routedParameter=bar
-    return make_response("unkown_route")  # $HttpResponse
+# View
+#
+# see https://flask.palletsprojects.com/en/1.1.x/views/#basic-principle
+
+from flask.views import View
+
+
+class ShowUser(View):
+
+    def dispatch_request(self, user_id):  # $ requestHandler routedParameter=user_id
+        return "user_id: {}".format(user_id)  # $ HttpResponse
+
+app.add_url_rule("/basic/user/<int:user_id>", view_func=ShowUser.as_view('show_user'))  # $routeSetup="/basic/user/<int:user_id>"
+
+
+class WithoutKnownRoute1(View):
+    # For handler without known route, treat all parameters as routed parameters
+    # (accepting that there might be a few FPs)
+    def dispatch_request(self, foo, not_routed=42):  # $ requestHandler routedParameter=foo SPURIOUS: routedParameter=not_routed
+        pass
+
+
+# MethodView
+#
+# see https://flask.palletsprojects.com/en/1.1.x/views/#method-views-for-apis
+
+from flask.views import MethodView
+
+
+class UserAPI(MethodView):
+
+    def get(self, user_id):  # $ requestHandler routedParameter=user_id
+        if user_id is None:
+            # return a list of users
+            pass
+        else:
+            # expose a single user
+            pass
+
+    def post(self):  # $ requestHandler
+        # create a new user
+        pass
+
+    def delete(self, user_id):  # $ requestHandler routedParameter=user_id
+        # delete a single user
+        pass
+
+    def put(self, user_id):  # $ requestHandler routedParameter=user_id
+        # update a single user
+        pass
+
+
+user_view = UserAPI.as_view("user_api")
+app.add_url_rule("/users/", defaults={"user_id": None}, view_func=user_view, methods=["GET",]) # $routeSetup="/users/"
+app.add_url_rule("/users/", view_func=user_view, methods=["POST",]) # $routeSetup="/users/"
+app.add_url_rule("/users/<int:user_id>", view_func=user_view, methods=["GET", "PUT", "DELETE"]) # $routeSetup="/users/<int:user_id>"
+
+
+class WithoutKnownRoute2(MethodView):
+    # For handler without known route, treat all parameters as routed parameters
+    # (accepting that there might be a few FPs)
+    def get(self, foo, not_routed=42):  # $ requestHandler routedParameter=foo SPURIOUS: routedParameter=not_routed
+        pass
 
 
 if __name__ == "__main__":