Django: Model any class used in django route setup as view class

Author: RasmusWL

…1-django-view-function-with-decorator.md …-notes/2021-12-21-django-improvements.mdpython/change-notes/2021-12-21-django-view-function-with-decorator.md renamed to python/change-notes/2021-12-21-django-improvements.md python/change-notes/2021-12-21-django-view-function-with-decorator.md renamed to python/change-notes/2021-12-21-django-improvements.md

@@ -1,2 +1,3 @@
 lgtm,codescanning
 * Improved modeling of `django` to recognize request handlers functions that are decorated (for example with `django.views.decorators.http.require_GET`). This leads to more sources of remote user input (`RemoteFlowSource`), since we correctly identify the first parameter as being passed a django request.
+* Improved modeling of django View classes. We now consider any class using in a routing setup with `<class>.as_view()` as django view class. This leads to more sources of remote user input (`RemoteFlowSource`), since we correctly identify the first parameter as being passed a django request.

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

@@ -2024,18 +2024,8 @@ private module Django {
     result = djangoRouteHandlerFunctionTracker(DataFlow::TypeTracker::end(), func)
   }
 
-  /** A django View class defined in project code. */
-  class DjangoViewClassDef extends Class {
-    DjangoViewClassDef() { this.getABase() = django::views::generic::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() = HTTP::httpVerbLower()
-    }
-
+  /** A class that might be a django View class. */
+  class PossibleDjangoViewClass extends Class {
     /** Gets a reference to this class. */
     private DataFlow::Node getARef(DataFlow::TypeTracker t) {
       t.start() and
@@ -2070,6 +2060,37 @@ private module Django {
     DataFlow::Node asViewResult() { result = asViewResult(DataFlow::TypeTracker::end()) }
   }
 
+  /** A class that we consider a django View class. */
+  abstract class DjangoViewClass extends PossibleDjangoViewClass {
+    /** 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() = HTTP::httpVerbLower()
+    }
+  }
+
+  /**
+   * A class that is used in a route-setup, with `<class>.as_view()`, therefore being
+   * considered a django View class.
+   */
+  class DjangoViewClassFromRouteSetup extends DjangoViewClass {
+    DjangoViewClassFromRouteSetup() {
+      exists(DjangoRouteSetup setup | setup.getViewArg() = this.asViewResult())
+    }
+  }
+
+  /**
+   * A class that has a super-type which is a django View class, therefore also
+   * becoming a django View class.
+   */
+  class DjangoViewClassFromSuperClass extends DjangoViewClass {
+    DjangoViewClassFromSuperClass() {
+      this.getABase() = django::views::generic::View::subclassRef().asExpr()
+    }
+  }
+
   /**
    * A function that is a django route handler, meaning it handles incoming requests
    * with the django framework.
@@ -2078,7 +2099,7 @@ private module Django {
     DjangoRouteHandler() {
       exists(DjangoRouteSetup route | route.getViewArg() = djangoRouteHandlerFunctionTracker(this))
       or
-      any(DjangoViewClassDef vc).getARequestHandler() = this
+      any(DjangoViewClass vc).getARequestHandler() = this
     }
 
     /** Gets the index of the request parameter. */
@@ -2102,7 +2123,7 @@ private module Django {
     final override DjangoRouteHandler getARequestHandler() {
       djangoRouteHandlerFunctionTracker(result) = getViewArg()
       or
-      exists(DjangoViewClassDef vc |
+      exists(DjangoViewClass vc |
         getViewArg() = vc.asViewResult() and
         result = vc.getARequestHandler()
       )
@@ -2113,7 +2134,7 @@ private module Django {
   private class DjangoViewClassHandlerWithoutKnownRoute extends HTTP::Server::RequestHandler::Range,
     DjangoRouteHandler {
     DjangoViewClassHandlerWithoutKnownRoute() {
-      exists(DjangoViewClassDef vc | vc.getARequestHandler() = this) and
+      exists(DjangoViewClass vc | vc.getARequestHandler() = this) and
       not exists(DjangoRouteSetup setup | setup.getARequestHandler() = this)
     }
 

python/ql/test/experimental/library-tests/frameworks/django-v2-v3/routing_test.py

@@ -144,7 +144,7 @@ def with_decorator(request, foo):  # $ requestHandler routedParameter=foo
 class UnknownViewSubclass(UnknownViewSuperclass):
     # Although we don't know for certain that this class is a django view class, the fact that it's
     # used with `as_view()` in the routing setup should be enough that we treat it as such.
-    def get(self, request): # $ MISSING: requestHandler
+    def get(self, request): # $ requestHandler
         pass
 
 urlpatterns = [