Python: Django: Handle Class-based views

Author: RasmusWL

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

@@ -6,10 +6,10 @@ import semmle.python.web.Http
 // a FunctionValue, so we can't use `FunctionValue.getArgumentForCall`
 // https://github.com/django/django/blob/master/django/urls/conf.py#L76
 abstract class DjangoRoute extends CallNode {
-    FunctionValue getViewFunction() {
-        result = this.getArg(1).pointsTo()
+    DjangoViewHandler getViewHandler() {
+        result = view_handler_from_view_arg(this.getArg(1))
         or
-        result = this.getArgByName("view").pointsTo()
+        result = view_handler_from_view_arg(this.getArgByName("view"))
     }
 
     abstract string getANamedArgument();
@@ -21,6 +21,60 @@ abstract class DjangoRoute extends CallNode {
     abstract int getNumPositionalArguments();
 }
 
+/**
+ * For function based views -- also see `DjangoClassBasedViewHandler`
+ * https://docs.djangoproject.com/en/1.11/topics/http/views/
+ * https://docs.djangoproject.com/en/3.0/topics/http/views/
+ */
+class DjangoViewHandler extends PythonFunctionValue {
+
+    /** Gets the index of the 'request' argument */
+    int getRequestArgIndex() {
+        result = 0
+    }
+}
+
+/**
+ * Class based views
+ * https://docs.djangoproject.com/en/1.11/topics/class-based-views/
+ * https://docs.djangoproject.com/en/3.0/topics/class-based-views/
+ */
+private class DjangoViewClass extends ClassValue {
+    DjangoViewClass() {
+        Value::named("django.views.generic.View") = this.getASuperType()
+        or
+        Value::named("django.views.View") = this.getASuperType()
+    }
+}
+
+class DjangoClassBasedViewHandler extends DjangoViewHandler {
+    DjangoClassBasedViewHandler() {
+        exists(DjangoViewClass cls |
+            cls.lookup(httpVerbLower()) = this
+        )
+    }
+
+    override int getRequestArgIndex() {
+        // due to `self` being the first parameter
+        result = 1
+    }
+}
+
+/**
+ * Gets the function that will handle requests when `view_arg` is used as the view argument to a
+ * django route. That is, this methods handles Class-based Views and its `as_view()` function.
+ */
+private DjangoViewHandler view_handler_from_view_arg(ControlFlowNode view_arg) {
+    // Function-based view
+    result = view_arg.pointsTo()
+    or
+    // Class-based view
+    exists(ClassValue cls |
+        cls = view_arg.(CallNode).getFunction().(AttrNode).getObject("as_view").pointsTo() and
+        result = cls.lookup(httpVerbLower())
+    )
+}
+
 // We need this "dummy" class, since otherwise the regex argument would not be considered
 // a regex (RegexString is abstract)
 class DjangoRouteRegex extends RegexString {

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

@@ -39,60 +39,35 @@ class DjangoQueryDict extends TaintKind {
     }
 }
 
-abstract class DjangoRequestSource extends HttpRequestTaintSource {
-    override string toString() { result = "Django request source" }
-
-    override predicate isSourceOf(TaintKind kind) { kind instanceof DjangoRequest }
-}
-
-/**
- * Function based views
- * https://docs.djangoproject.com/en/1.11/topics/http/views/
- * https://docs.djangoproject.com/en/3.0/topics/http/views/
- */
-private class DjangoFunctionBasedViewRequestArgument extends DjangoRequestSource {
-    DjangoFunctionBasedViewRequestArgument() {
-        exists(DjangoRoute route, FunctionValue view |
-            route.getViewFunction() = view and
-            this = view.getScope().getArg(0).asName().getAFlowNode()
+/** A Django request parameter */
+class DjangoRequestSource extends HttpRequestTaintSource {
+    DjangoRequestSource() {
+        exists(DjangoRoute route, DjangoViewHandler view, int request_arg_index |
+            route.getViewHandler() = view and
+            request_arg_index = view.getRequestArgIndex() and
+            this = view.getScope().getArg(request_arg_index).asName().getAFlowNode()
         )
     }
-}
 
-/**
- * Class based views
- * https://docs.djangoproject.com/en/1.11/topics/class-based-views/
- * https://docs.djangoproject.com/en/3.0/topics/class-based-views/
- */
-private class DjangoView extends ClassValue {
-    DjangoView() {
-        Value::named("django.views.generic.View") = this.getASuperType()
-        or
-        Value::named("django.views.View") = this.getASuperType()
-    }
-}
-
-private FunctionValue djangoViewHttpMethod() {
-    exists(DjangoView view | view.lookup(httpVerbLower()) = result)
-}
+    override string toString() { result = "Django request source" }
 
-class DjangoClassBasedViewRequestArgument extends DjangoRequestSource {
-    DjangoClassBasedViewRequestArgument() {
-        this = djangoViewHttpMethod().getScope().getArg(1).asName().getAFlowNode()
-    }
+    override predicate isSourceOf(TaintKind kind) { kind instanceof DjangoRequest }
 }
 
 /** An argument specified in a url routing table */
 class DjangoRequestParameter extends HttpRequestTaintSource {
     DjangoRequestParameter() {
-        exists(DjangoRoute route, Function f |
-            f = route.getViewFunction().getScope() |
+        exists(DjangoRoute route, Function f, DjangoViewHandler view, int request_arg_index |
+            route.getViewHandler() = view and
+            request_arg_index = view.getRequestArgIndex() and
+            f = view.getScope()
+        |
             this.(ControlFlowNode).getNode() = f.getArgByName(route.getANamedArgument())
             or
             exists(int i | i >= 0 |
                 i < route.getNumPositionalArguments() and
                 // +1 because first argument is always the request
-                this.(ControlFlowNode).getNode() = f.getArg(i+1)
+                this.(ControlFlowNode).getNode() = f.getArg(request_arg_index + 1 + i)
             )
         )
     }

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

@@ -13,7 +13,9 @@
 | views_1x.py:15:21:15:27 | request | django.request.HttpRequest |
 | views_1x.py:19:21:19:27 | request | django.request.HttpRequest |
 | views_1x.py:29:20:29:26 | request | django.request.HttpRequest |
+| views_1x.py:29:29:29:37 | untrusted | externally controlled string |
 | views_1x.py:35:19:35:25 | request | django.request.HttpRequest |
+| views_1x.py:35:28:35:36 | untrusted | externally controlled string |
 | views_1x.py:39:19:39:25 | request | django.request.HttpRequest |
 | views_1x.py:39:28:39:38 | page_number | externally controlled string |
 | views_1x.py:44:24:44:30 | request | django.request.HttpRequest |
@@ -29,7 +31,9 @@
 | views_2x_3x.py:15:21:15:27 | request | django.request.HttpRequest |
 | views_2x_3x.py:19:21:19:27 | request | django.request.HttpRequest |
 | views_2x_3x.py:29:20:29:26 | request | django.request.HttpRequest |
+| views_2x_3x.py:29:29:29:37 | untrusted | externally controlled string |
 | views_2x_3x.py:35:19:35:25 | request | django.request.HttpRequest |
+| views_2x_3x.py:35:28:35:36 | untrusted | externally controlled string |
 | views_2x_3x.py:39:19:39:25 | request | django.request.HttpRequest |
 | views_2x_3x.py:39:28:39:38 | page_number | externally controlled string |
 | views_2x_3x.py:44:24:44:30 | request | django.request.HttpRequest |

python/ql/test/library-tests/web/django/views_1x.py

@@ -25,13 +25,13 @@ def http_resp_write(request):
 class Foo(object):
     # Note: since Foo is used as the super type in a class view, it will be able to handle requests.
 
-    # TODO: Currently we don't flag `untrusted` as a DjangoRequestParameter
+
     def post(self, request, untrusted):
         return HttpResponse('Foo post: {}'.format(untrusted))
 
 
 class ClassView(View, Foo):
-    # TODO: Currently we don't flag `untrusted` as a DjangoRequestParameter
+
     def get(self, request, untrusted):
         return HttpResponse('ClassView get: {}'.format(untrusted))
 

python/ql/test/library-tests/web/django/views_2x_3x.py

@@ -25,13 +25,13 @@ def http_resp_write(request):
 class Foo(object):
     # Note: since Foo is used as the super type in a class view, it will be able to handle requests.
 
-    # TODO: Currently we don't flag `untrusted` as a DjangoRequestParameter
+
     def post(self, request, untrusted):
         return HttpResponse('Foo post: {}'.format(untrusted))
 
 
 class ClassView(View, Foo):
-    # TODO: Currently we don't flag `untrusted` as a DjangoRequestParameter
+
     def get(self, request, untrusted):
         return HttpResponse('ClassView get: {}'.format(untrusted))