Python: Model taint of self.request on django view class

Author: RasmusWL

python/change-notes/2021-12-21-django-improvements.md

@@ -1,3 +1,4 @@
 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.
+* Improved modeling of `django`, so for View classes we now model `self.request`, `self.args`, and `self.kwargs` as sources of remote user input (`RemoteFlowSource`).

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

@@ -2069,6 +2069,29 @@ private module Django {
       result = this.getAMethod() and
       result.getName() = HTTP::httpVerbLower()
     }
+
+    /**
+     * Gets a reference to instances of this class, originating from a self parameter of
+     * a method defined on this class.
+     *
+     * Note: TODO: This doesn't take MRO into account
+     * Note: TODO: This doesn't take staticmethod/classmethod into account
+     */
+    private DataFlow::Node getASelfRef(DataFlow::TypeTracker t) {
+      t.start() and
+      result.(DataFlow::ParameterNode).getParameter() = this.getAMethod().getArg(0)
+      or
+      exists(DataFlow::TypeTracker t2 | result = this.getASelfRef(t2).track(t2, t))
+    }
+
+    /**
+     * Gets a reference to instances of this class, originating from a self parameter of
+     * a method defined on this class.
+     *
+     * Note: TODO: This doesn't take MRO into account
+     * Note: TODO: This doesn't take staticmethod/classmethod into account
+     */
+    DataFlow::Node getASelfRef() { result = this.getASelfRef(DataFlow::TypeTracker::end()) }
   }
 
   /**
@@ -2307,6 +2330,46 @@ private module Django {
     override string getSourceType() { result = "django.http.request.HttpRequest" }
   }
 
+  /**
+   * A read of the `request` attribute on a reference to an instance of a View class,
+   * which is the request being processed currently.
+   *
+   * See https://docs.djangoproject.com/en/3.1/topics/class-based-views/generic-display/#dynamic-filtering
+   */
+  private class DjangoViewClassRequestAttributeRead extends django::http::request::HttpRequest::InstanceSource,
+    RemoteFlowSource::Range, DataFlow::Node {
+    DjangoViewClassRequestAttributeRead() {
+      exists(DataFlow::AttrRead read | this = read |
+        read.getObject() = any(DjangoViewClass vc).getASelfRef() and
+        read.getAttributeName() = "request"
+      )
+    }
+
+    override string getSourceType() {
+      result = "django.http.request.HttpRequest (attribute on self in View class)"
+    }
+  }
+
+  /**
+   * A read of the `args` or `kwargs` attribute on a reference to an instance of a View class,
+   * which contains the routed parameters captured from the URL route.
+   *
+   * See https://docs.djangoproject.com/en/3.1/topics/class-based-views/generic-display/#dynamic-filtering
+   */
+  private class DjangoViewClassRoutedParamsAttributeRead extends RemoteFlowSource::Range,
+    DataFlow::Node {
+    DjangoViewClassRoutedParamsAttributeRead() {
+      exists(DataFlow::AttrRead read | this = read |
+        read.getObject() = any(DjangoViewClass vc).getASelfRef() and
+        read.getAttributeName() in ["args", "kwargs"]
+      )
+    }
+
+    override string getSourceType() {
+      result = "django routed param from attribute on self in View class"
+    }
+  }
+
   private class DjangoHttpRequstAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
       nodeFrom = django::http::request::HttpRequest::instance() and

python/ql/test/experimental/library-tests/frameworks/django-v2-v3/TestTaint.expected

@@ -80,9 +80,9 @@
 | taint_test.py:145 | ok   | test_taint | request.get_signed_cookie(..) |
 | taint_test.py:149 | fail | test_taint | request.get_signed_cookie(..) |
 | taint_test.py:150 | fail | test_taint | request.get_signed_cookie(..) |
-| taint_test.py:157 | fail | some_method | self.request |
-| taint_test.py:158 | fail | some_method | self.request.GET["key"] |
-| taint_test.py:160 | fail | some_method | self.args |
-| taint_test.py:161 | fail | some_method | self.args[0] |
-| taint_test.py:163 | fail | some_method | self.kwargs |
-| taint_test.py:164 | fail | some_method | self.kwargs["key"] |
+| taint_test.py:157 | ok   | some_method | self.request |
+| taint_test.py:158 | ok   | some_method | self.request.GET["key"] |
+| taint_test.py:160 | ok   | some_method | self.args |
+| taint_test.py:161 | ok   | some_method | self.args[0] |
+| taint_test.py:163 | ok   | some_method | self.kwargs |
+| taint_test.py:164 | ok   | some_method | self.kwargs["key"] |