From 72ea4ff0dcd4dbe80ca9f3040fe655c7b875d289 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Mon, 18 May 2020 16:56:47 +0200
Subject: [PATCH 1/8] Python: Add more tests of django responses

They clearly shouldn't all be XSS sinks
---
 .../web/django/HttpResponseSinks.expected     | 10 +++++
 .../test/library-tests/web/django/views_1x.py | 26 ++++++++++++-
 .../library-tests/web/django/views_2x_3x.py   | 38 ++++++++++---------
 .../Security/lib/django/http/__init__.py      |  2 +-
 .../Security/lib/django/http/response.py      | 38 ++++++++++++++++++-
 5 files changed, 93 insertions(+), 21 deletions(-)

diff --git a/python/ql/test/library-tests/web/django/HttpResponseSinks.expected b/python/ql/test/library-tests/web/django/HttpResponseSinks.expected
index 0180a8726e4..8ecde34d08d 100644
--- a/python/ql/test/library-tests/web/django/HttpResponseSinks.expected
+++ b/python/ql/test/library-tests/web/django/HttpResponseSinks.expected
@@ -8,6 +8,11 @@
 | views_1x.py:45:25:45:70 | django.Response(...) | externally controlled string |
 | views_1x.py:66:25:66:55 | django.Response(...) | externally controlled string |
 | views_1x.py:75:25:75:33 | django.Response(...) | externally controlled string |
+| views_1x.py:85:25:85:55 | django.Response(...) | externally controlled string |
+| views_1x.py:90:25:90:33 | django.Response(...) | externally controlled string |
+| views_1x.py:94:25:94:58 | django.Response(...) | externally controlled string |
+| views_1x.py:99:33:99:55 | django.Response(...) | externally controlled string |
+| views_1x.py:103:33:103:55 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:8:25:8:63 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:12:25:12:52 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:16:25:16:53 | django.Response(...) | externally controlled string |
@@ -21,3 +26,8 @@
 | views_2x_3x.py:82:25:82:69 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:85:25:85:64 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:88:25:88:32 | django.Response(...) | externally controlled string |
+| views_2x_3x.py:106:25:106:55 | django.Response(...) | externally controlled string |
+| views_2x_3x.py:111:25:111:33 | django.Response(...) | externally controlled string |
+| views_2x_3x.py:115:25:115:58 | django.Response(...) | externally controlled string |
+| views_2x_3x.py:120:33:120:55 | django.Response(...) | externally controlled string |
+| views_2x_3x.py:124:33:124:55 | django.Response(...) | externally controlled string |
diff --git a/python/ql/test/library-tests/web/django/views_1x.py b/python/ql/test/library-tests/web/django/views_1x.py
index 068f8246210..abc3b716a2f 100644
--- a/python/ql/test/library-tests/web/django/views_1x.py
+++ b/python/ql/test/library-tests/web/django/views_1x.py
@@ -1,6 +1,6 @@
 """test of views for Django 1.x"""
 from django.conf.urls import patterns, url
-from django.http.response import HttpResponse
+from django.http.response import HttpResponse, HttpResponseRedirect, JsonResponse, HttpResponseNotFound
 from django.views.generic import View
 
 
@@ -77,3 +77,27 @@ def kw_args(request):
 urlpatterns = [
     url(view=kw_args, regex=r'^kw_args$')
 ]
+
+# Not an XSS sink, since the Content-Type is not "text/html"
+# FP reported in https://github.com/github/codeql-python-team/issues/38
+def fp_json_response(request):
+    # implicitly sets Content-Type to "application/json"
+    return JsonResponse({"foo": request.GET.get("foo")})
+
+# Not an XSS sink, since the Content-Type is not "text/html"
+def fp_manual_json_response(request):
+    json_data = '{"json": "{}"}'.format(request.GET.get("foo"))
+    return HttpResponse(json_data, content_type="application/json")
+
+# Not an XSS sink, since the Content-Type is not "text/html"
+def fp_manual_content_type(reuqest):
+    return HttpResponse('<img src="0" onerror="alert(1)">', content_type="text/plain")
+
+# XSS FP reported in https://github.com/github/codeql/issues/3466
+# Note: This should be a open-redirect sink, but not a XSS sink.
+def fp_redirect(request):
+    return HttpResponseRedirect(request.GET.get("next"))
+
+# Ensure that subclasses are still vuln to XSS
+def tp_not_found(request):
+    return HttpResponseNotFound(request.GET.get("name"))
diff --git a/python/ql/test/library-tests/web/django/views_2x_3x.py b/python/ql/test/library-tests/web/django/views_2x_3x.py
index 50fe6d638fe..ce4a565c7a0 100644
--- a/python/ql/test/library-tests/web/django/views_2x_3x.py
+++ b/python/ql/test/library-tests/web/django/views_2x_3x.py
@@ -1,6 +1,6 @@
 """testing views for Django 2.x and 3.x"""
 from django.urls import path, re_path
-from django.http import HttpResponse
+from django.http import HttpResponse, HttpResponseRedirect, JsonResponse, HttpResponseNotFound
 from django.views import View
 
 
@@ -99,24 +99,26 @@ urlpatterns = [
 ]
 
 
-################################################################################
+# Not an XSS sink, since the Content-Type is not "text/html"
+# FP reported in https://github.com/github/codeql-python-team/issues/38
+def fp_json_response(request):
+    # implicitly sets Content-Type to "application/json"
+    return JsonResponse({"foo": request.GET.get("foo")}) # TODO
 
+# Not an XSS sink, since the Content-Type is not "text/html"
+def fp_manual_json_response(request):
+    json_data = '{"json": "{}"}'.format(request.GET.get("foo"))
+    return HttpResponse(json_data, content_type="application/json") # TODO
 
-# We should abort if a decorator is used. As demonstrated below, anything might happen
+# Not an XSS sink, since the Content-Type is not "text/html"
+def fp_manual_content_type(reuqest):
+    return HttpResponse('<img src="0" onerror="alert(1)">', content_type="text/plain") # TODO
 
-# def reverse_kwargs(f):
-#     @wraps(f)
-#     def f_(*args, **kwargs):
-#         new_kwargs = dict()
-#         for key, value in kwargs.items():
-#             new_kwargs[key[::-1]] = value
-#         return f(*args, **new_kwargs)
-#     return f_
+# XSS FP reported in https://github.com/github/codeql/issues/3466
+# Note: This should be a open-redirect sink, but not a XSS sink.
+def fp_redirect(request):
+    return HttpResponseRedirect(request.GET.get("next")) # TODO
 
-# @reverse_kwargs
-# def decorators_can_do_anything(request, oof, foo=None):
-#     return HttpResponse('This is a mess'[::-1])
-
-# urlpatterns = [
-#     path('rev/<foo>', decorators_can_do_anything),
-# ]
+# Ensure that subclasses are still vuln to XSS
+def tp_not_found(request):
+    return HttpResponseNotFound(request.GET.get("name"))
diff --git a/python/ql/test/query-tests/Security/lib/django/http/__init__.py b/python/ql/test/query-tests/Security/lib/django/http/__init__.py
index 962077dbad6..f2ac6d2c55b 100644
--- a/python/ql/test/query-tests/Security/lib/django/http/__init__.py
+++ b/python/ql/test/query-tests/Security/lib/django/http/__init__.py
@@ -1,2 +1,2 @@
-from .response import HttpResponse
+from .response import *
 from .request import HttpRequest
diff --git a/python/ql/test/query-tests/Security/lib/django/http/response.py b/python/ql/test/query-tests/Security/lib/django/http/response.py
index ae101562df4..96cc2bda9a9 100644
--- a/python/ql/test/query-tests/Security/lib/django/http/response.py
+++ b/python/ql/test/query-tests/Security/lib/django/http/response.py
@@ -1,2 +1,38 @@
-class HttpResponse(object):
+class HttpResponseBase(object):
+    status_code = 200
+
+
+class HttpResponse(HttpResponseBase):
     pass
+
+
+class HttpResponseRedirectBase(HttpResponse):
+    pass
+
+
+class HttpResponsePermanentRedirect(HttpResponseRedirectBase):
+    status_code = 301
+
+
+class HttpResponseRedirect(HttpResponseRedirectBase):
+    status_code = 302
+
+
+class HttpResponseNotFound(HttpResponse):
+    status_code = 404
+
+
+class JsonResponse(HttpResponse):
+
+    def __init__(
+        self,
+        data,
+        encoder=...,
+        safe=True,
+        json_dumps_params=None,
+        **kwargs
+    ):
+        # fake code to represent what is going on :)
+        kwargs.setdefault("content_type", "application/json")
+        data = str(data)
+        super().__init__(content=data, **kwargs)

From fa08676a1db08a12ba1c97102268adaff1af8209 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Mon, 18 May 2020 17:35:31 +0200
Subject: [PATCH 2/8] Python: Proper redirect taint sinks for Django

Also a major restructuring of the code. A bit controversial since it
renames/moves classes that are already public.

Fixes https://github.com/github/codeql/issues/3466
---
 .../src/semmle/python/web/django/Redirect.qll | 27 +++++++++--
 .../src/semmle/python/web/django/Response.qll | 39 +++++-----------
 .../src/semmle/python/web/django/Shared.qll   | 45 +++++++++++++++----
 .../web/django/HttpRedirectSinks.expected     |  6 ++-
 .../web/django/HttpResponseSinks.expected     |  2 -
 .../library-tests/web/django/views_2x_3x.py   |  2 +-
 6 files changed, 74 insertions(+), 47 deletions(-)

diff --git a/python/ql/src/semmle/python/web/django/Redirect.qll b/python/ql/src/semmle/python/web/django/Redirect.qll
index a550088eaf6..ca28a4a32d2 100644
--- a/python/ql/src/semmle/python/web/django/Redirect.qll
+++ b/python/ql/src/semmle/python/web/django/Redirect.qll
@@ -11,10 +11,29 @@ private import semmle.python.web.django.Shared
 private import semmle.python.web.Http
 
 /**
- * Represents an argument to the `django.redirect` function.
+ * The URL argument for a call to the `django.shortcuts.redirect` function.
  */
-class DjangoRedirect extends HttpRedirectTaintSink {
-    override string toString() { result = "django.redirect" }
+class DjangoShortcutsRedirectSink extends HttpRedirectTaintSink {
+    override string toString() { result = "DjangoShortcutsRedirectSink" }
 
-    DjangoRedirect() { this = redirect().getACall().getAnArg() }
+    DjangoShortcutsRedirectSink() {
+        this = Value::named("django.shortcuts.redirect").(FunctionValue).getArgumentForCall(_, 0)
+    }
+}
+
+/**
+ * The URL argument when instantiating a Django Redirect Response.
+ */
+class DjangoRedirectResponseSink extends HttpRedirectTaintSink {
+    DjangoRedirectResponseSink() {
+        exists(CallNode call |
+            call = any(DjangoRedirectResponse rr).getACall()
+        |
+            this = call.getArg(0)
+            or
+            this = call.getArgByName("redirect_to")
+        )
+    }
+
+    override string toString() { result = "DjangoRedirectResponseSink" }
 }
diff --git a/python/ql/src/semmle/python/web/django/Response.qll b/python/ql/src/semmle/python/web/django/Response.qll
index dc6a3634440..c9e78130d36 100644
--- a/python/ql/src/semmle/python/web/django/Response.qll
+++ b/python/ql/src/semmle/python/web/django/Response.qll
@@ -4,38 +4,20 @@ import semmle.python.security.strings.Basic
 private import semmle.python.web.django.Shared
 private import semmle.python.web.Http
 
-/**
- * A django.http.response.Response object
- * This isn't really a "taint", but we use the value tracking machinery to
- * track the flow of response objects.
- */
-class DjangoResponse extends TaintKind {
-    DjangoResponse() { this = "django.response.HttpResponse" }
+/** INTERNAL class used for tracking a django response object. */
+private class DjangoResponseKind extends TaintKind {
+    DjangoResponseKind() { this = "django.response.HttpResponse" }
 }
 
-private ClassValue theDjangoHttpResponseClass() {
-    (
-        // version 1.x
-        result = Value::named("django.http.response.HttpResponse")
-        or
-        // version 2.x
-        // https://docs.djangoproject.com/en/2.2/ref/request-response/#httpresponse-objects
-        result = Value::named("django.http.HttpResponse")
-    ) and
-    // TODO: does this do anything? when could they be the same???
-    not result = theDjangoHttpRedirectClass()
-}
-
-/** internal class used for tracking a django response. */
+/** INTENRAL taint-source used for tracking a django response. */
 private class DjangoResponseSource extends TaintSource {
     DjangoResponseSource() {
-        exists(ClassValue cls |
-            cls.getASuperType() = theDjangoHttpResponseClass() and
+        exists(DjangoXSSVulnResponse cls |
             cls.getACall() = this
         )
     }
 
-    override predicate isSourceOf(TaintKind kind) { kind instanceof DjangoResponse }
+    override predicate isSourceOf(TaintKind kind) { kind instanceof DjangoResponseKind }
 
     override string toString() { result = "django.http.response.HttpResponse" }
 }
@@ -45,7 +27,7 @@ class DjangoResponseWrite extends HttpResponseTaintSink {
     DjangoResponseWrite() {
         exists(AttrNode meth, CallNode call |
             call.getFunction() = meth and
-            any(DjangoResponse response).taints(meth.getObject("write")) and
+            any(DjangoResponseKind response).taints(meth.getObject("write")) and
             this = call.getArg(0)
         )
     }
@@ -58,9 +40,8 @@ class DjangoResponseWrite extends HttpResponseTaintSink {
 /** An argument to initialization of a django response, which is vulnerable to external data (xss) */
 class DjangoResponseContent extends HttpResponseTaintSink {
     DjangoResponseContent() {
-        exists(CallNode call, ClassValue cls |
-            cls.getASuperType() = theDjangoHttpResponseClass() and
-            call.getFunction().pointsTo(cls)
+        exists(CallNode call, DjangoXSSVulnResponse cls |
+            call = cls.getACall()
         |
             call.getArg(0) = this
             or
@@ -75,7 +56,7 @@ class DjangoResponseContent extends HttpResponseTaintSink {
 
 class DjangoCookieSet extends CookieSet, CallNode {
     DjangoCookieSet() {
-        any(DjangoResponse r).taints(this.getFunction().(AttrNode).getObject("set_cookie"))
+        any(DjangoResponseKind r).taints(this.getFunction().(AttrNode).getObject("set_cookie"))
     }
 
     override string toString() { result = CallNode.super.toString() }
diff --git a/python/ql/src/semmle/python/web/django/Shared.qll b/python/ql/src/semmle/python/web/django/Shared.qll
index 669dc5712df..ef3d353b2d2 100644
--- a/python/ql/src/semmle/python/web/django/Shared.qll
+++ b/python/ql/src/semmle/python/web/django/Shared.qll
@@ -1,12 +1,39 @@
 import python
 
-/** django.shortcuts.redirect */
-FunctionValue redirect() { result = Value::named("django.shortcuts.redirect") }
-
-ClassValue theDjangoHttpRedirectClass() {
-    // version 1.x
-    result = Value::named("django.http.response.HttpResponseRedirectBase")
-    or
-    // version 2.x
-    result = Value::named("django.http.HttpResponseRedirectBase")
+/** A Class that is a Django Response (subclass of `django.http.HttpResponse`). */
+class DjangoResponse extends ClassValue {
+    DjangoResponse() {
+        exists(ClassValue base |
+            // version 1.x
+            base = Value::named("django.http.response.HttpResponse")
+            or
+            // version 2.x and 3.x
+            // https://docs.djangoproject.com/en/2.2/ref/request-response/#httpresponse-objects
+            base = Value::named("django.http.HttpResponse")
+        |
+            this.getASuperType() = base
+        )
+    }
+}
+
+/** A Class that is a Django Redirect Response (subclass of `django.http.HttpResponseRedirectBase`). */
+class DjangoRedirectResponse extends DjangoResponse {
+    DjangoRedirectResponse() {
+        exists(ClassValue base |
+            // version 1.x
+            base = Value::named("django.http.response.HttpResponseRedirectBase")
+            or
+            // version 2.x and 3.x
+            base = Value::named("django.http.HttpResponseRedirectBase")
+        |
+            this.getASuperType() = base
+        )
+    }
+}
+
+/** A Class that is a Django Response, and is vulnerable to XSS. */
+class DjangoXSSVulnResponse extends DjangoResponse {
+    DjangoXSSVulnResponse() {
+        not this instanceof DjangoRedirectResponse
+    }
 }
diff --git a/python/ql/test/library-tests/web/django/HttpRedirectSinks.expected b/python/ql/test/library-tests/web/django/HttpRedirectSinks.expected
index c47548dd3a5..1c9bcb0cfd5 100644
--- a/python/ql/test/library-tests/web/django/HttpRedirectSinks.expected
+++ b/python/ql/test/library-tests/web/django/HttpRedirectSinks.expected
@@ -1,2 +1,4 @@
-| test_1x.py:13:21:13:24 | django.redirect | externally controlled string |
-| test_2x_3x.py:13:21:13:24 | django.redirect | externally controlled string |
+| test_1x.py:13:21:13:24 | DjangoShortcutsRedirectSink | externally controlled string |
+| test_2x_3x.py:13:21:13:24 | DjangoShortcutsRedirectSink | externally controlled string |
+| views_1x.py:99:33:99:55 | DjangoRedirectResponseSink | externally controlled string |
+| views_2x_3x.py:120:33:120:55 | DjangoRedirectResponseSink | externally controlled string |
diff --git a/python/ql/test/library-tests/web/django/HttpResponseSinks.expected b/python/ql/test/library-tests/web/django/HttpResponseSinks.expected
index 8ecde34d08d..c9c02bc9a12 100644
--- a/python/ql/test/library-tests/web/django/HttpResponseSinks.expected
+++ b/python/ql/test/library-tests/web/django/HttpResponseSinks.expected
@@ -11,7 +11,6 @@
 | views_1x.py:85:25:85:55 | django.Response(...) | externally controlled string |
 | views_1x.py:90:25:90:33 | django.Response(...) | externally controlled string |
 | views_1x.py:94:25:94:58 | django.Response(...) | externally controlled string |
-| views_1x.py:99:33:99:55 | django.Response(...) | externally controlled string |
 | views_1x.py:103:33:103:55 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:8:25:8:63 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:12:25:12:52 | django.Response(...) | externally controlled string |
@@ -29,5 +28,4 @@
 | views_2x_3x.py:106:25:106:55 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:111:25:111:33 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:115:25:115:58 | django.Response(...) | externally controlled string |
-| views_2x_3x.py:120:33:120:55 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:124:33:124:55 | django.Response(...) | externally controlled string |
diff --git a/python/ql/test/library-tests/web/django/views_2x_3x.py b/python/ql/test/library-tests/web/django/views_2x_3x.py
index ce4a565c7a0..72371b789e9 100644
--- a/python/ql/test/library-tests/web/django/views_2x_3x.py
+++ b/python/ql/test/library-tests/web/django/views_2x_3x.py
@@ -117,7 +117,7 @@ def fp_manual_content_type(reuqest):
 # XSS FP reported in https://github.com/github/codeql/issues/3466
 # Note: This should be a open-redirect sink, but not a XSS sink.
 def fp_redirect(request):
-    return HttpResponseRedirect(request.GET.get("next")) # TODO
+    return HttpResponseRedirect(request.GET.get("next"))
 
 # Ensure that subclasses are still vuln to XSS
 def tp_not_found(request):

From 37743109853175bb2587b4d88019c177666303d9 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Mon, 18 May 2020 19:13:50 +0200
Subject: [PATCH 3/8] Python: Reduce FPs in Django due to bad XSS taint-sinks

Fixes https://github.com/github/codeql-python-team/issues/38
---
 .../src/semmle/python/web/django/Response.qll | 19 +++++---
 .../src/semmle/python/web/django/Shared.qll   | 46 +++++++++++++++----
 .../web/django/HttpResponseSinks.expected     |  8 +---
 .../test/library-tests/web/django/views_1x.py |  6 ++-
 .../library-tests/web/django/views_2x_3x.py   | 12 +++--
 .../Security/lib/django/http/response.py      | 12 ++++-
 6 files changed, 73 insertions(+), 30 deletions(-)

diff --git a/python/ql/src/semmle/python/web/django/Response.qll b/python/ql/src/semmle/python/web/django/Response.qll
index c9e78130d36..b2b2f2e162d 100644
--- a/python/ql/src/semmle/python/web/django/Response.qll
+++ b/python/ql/src/semmle/python/web/django/Response.qll
@@ -12,7 +12,7 @@ private class DjangoResponseKind extends TaintKind {
 /** INTENRAL taint-source used for tracking a django response. */
 private class DjangoResponseSource extends TaintSource {
     DjangoResponseSource() {
-        exists(DjangoXSSVulnResponse cls |
+        exists(DjangoXSSVulnerableResponse cls |
             cls.getACall() = this
         )
     }
@@ -40,12 +40,17 @@ class DjangoResponseWrite extends HttpResponseTaintSink {
 /** An argument to initialization of a django response, which is vulnerable to external data (xss) */
 class DjangoResponseContent extends HttpResponseTaintSink {
     DjangoResponseContent() {
-        exists(CallNode call, DjangoXSSVulnResponse cls |
-            call = cls.getACall()
-        |
-            call.getArg(0) = this
-            or
-            call.getArgByName("content") = this
+        exists(CallNode call, DjangoXSSVulnerableResponse cls |
+            call = cls.getACall() and
+            this = cls.getContentArg(call) and
+            (
+                not exists(cls.getContentTypeArg(call))
+                or
+                exists(StringValue s |
+                    cls.getContentTypeArg(call).pointsTo(s) and
+                    s.getText().indexOf("text/html") = 0
+                )
+            )
         )
     }
 
diff --git a/python/ql/src/semmle/python/web/django/Shared.qll b/python/ql/src/semmle/python/web/django/Shared.qll
index ef3d353b2d2..b4402723273 100644
--- a/python/ql/src/semmle/python/web/django/Shared.qll
+++ b/python/ql/src/semmle/python/web/django/Shared.qll
@@ -2,38 +2,64 @@ import python
 
 /** A Class that is a Django Response (subclass of `django.http.HttpResponse`). */
 class DjangoResponse extends ClassValue {
+    ClassValue base;
+
     DjangoResponse() {
-        exists(ClassValue base |
+        (
             // version 1.x
             base = Value::named("django.http.response.HttpResponse")
             or
             // version 2.x and 3.x
             // https://docs.djangoproject.com/en/2.2/ref/request-response/#httpresponse-objects
             base = Value::named("django.http.HttpResponse")
-        |
-            this.getASuperType() = base
-        )
+        ) and
+        this.getASuperType() = base
     }
 }
 
 /** A Class that is a Django Redirect Response (subclass of `django.http.HttpResponseRedirectBase`). */
 class DjangoRedirectResponse extends DjangoResponse {
     DjangoRedirectResponse() {
-        exists(ClassValue base |
+        exists(ClassValue redirect_base |
             // version 1.x
-            base = Value::named("django.http.response.HttpResponseRedirectBase")
+            redirect_base = Value::named("django.http.response.HttpResponseRedirectBase")
             or
             // version 2.x and 3.x
-            base = Value::named("django.http.HttpResponseRedirectBase")
+            redirect_base = Value::named("django.http.HttpResponseRedirectBase")
         |
-            this.getASuperType() = base
+            this.getASuperType() = redirect_base
         )
     }
 }
 
 /** A Class that is a Django Response, and is vulnerable to XSS. */
-class DjangoXSSVulnResponse extends DjangoResponse {
-    DjangoXSSVulnResponse() {
+class DjangoXSSVulnerableResponse extends DjangoResponse {
+    DjangoXSSVulnerableResponse() {
+        // We want to avoid FPs on subclasses that are not exposed to XSS, for example `JsonResponse`.
+        // The easiest way is to disregard any subclass that has a special `__init__` method.
+        // It's not guaranteed to remove all FPs, or not to generate FNs, but compared to our
+        // previous implementation that would treat 0-th argument to _any_ subclass as a sink,
+        // this gets us much closer to reality.
+        this.lookup("__init__") = base.lookup("__init__") and
         not this instanceof DjangoRedirectResponse
     }
+
+    // The reason these two method are defined in this class (and no in the Sink
+    // definition that uses this class), is that if we were to add support for `HttpResponseNotAllowed`
+    // it would make much more sense to add the custom logic in this class (or subclass), than to handle all of it
+    // in the sink definition.
+
+    /** Gets the `content` argument of a `call` to the constructor */
+    ControlFlowNode getContentArg(CallNode call) {
+        result = call.getArg(0)
+        or
+        result = call.getArgByName("content")
+    }
+
+    /** Gets the `content_type` argument of a `call` to the constructor */
+    ControlFlowNode getContentTypeArg(CallNode call) {
+        result = call.getArg(1)
+        or
+        result = call.getArgByName("content_type")
+    }
 }
diff --git a/python/ql/test/library-tests/web/django/HttpResponseSinks.expected b/python/ql/test/library-tests/web/django/HttpResponseSinks.expected
index c9c02bc9a12..7c9e583095f 100644
--- a/python/ql/test/library-tests/web/django/HttpResponseSinks.expected
+++ b/python/ql/test/library-tests/web/django/HttpResponseSinks.expected
@@ -8,10 +8,8 @@
 | views_1x.py:45:25:45:70 | django.Response(...) | externally controlled string |
 | views_1x.py:66:25:66:55 | django.Response(...) | externally controlled string |
 | views_1x.py:75:25:75:33 | django.Response(...) | externally controlled string |
-| views_1x.py:85:25:85:55 | django.Response(...) | externally controlled string |
-| views_1x.py:90:25:90:33 | django.Response(...) | externally controlled string |
-| views_1x.py:94:25:94:58 | django.Response(...) | externally controlled string |
 | views_1x.py:103:33:103:55 | django.Response(...) | externally controlled string |
+| views_1x.py:107:25:107:47 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:8:25:8:63 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:12:25:12:52 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:16:25:16:53 | django.Response(...) | externally controlled string |
@@ -25,7 +23,5 @@
 | views_2x_3x.py:82:25:82:69 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:85:25:85:64 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:88:25:88:32 | django.Response(...) | externally controlled string |
-| views_2x_3x.py:106:25:106:55 | django.Response(...) | externally controlled string |
-| views_2x_3x.py:111:25:111:33 | django.Response(...) | externally controlled string |
-| views_2x_3x.py:115:25:115:58 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:124:33:124:55 | django.Response(...) | externally controlled string |
+| views_2x_3x.py:128:25:128:47 | django.Response(...) | externally controlled string |
diff --git a/python/ql/test/library-tests/web/django/views_1x.py b/python/ql/test/library-tests/web/django/views_1x.py
index abc3b716a2f..f5476a13cef 100644
--- a/python/ql/test/library-tests/web/django/views_1x.py
+++ b/python/ql/test/library-tests/web/django/views_1x.py
@@ -98,6 +98,10 @@ def fp_manual_content_type(reuqest):
 def fp_redirect(request):
     return HttpResponseRedirect(request.GET.get("next"))
 
-# Ensure that subclasses are still vuln to XSS
+# Ensure that simple subclasses are still vuln to XSS
 def tp_not_found(request):
     return HttpResponseNotFound(request.GET.get("name"))
+
+# Ensure we still have a XSS sink when manually setting the content_type to HTML
+def tp_manual_response_type(request):
+    return HttpResponse(request.GET.get("name"), content_type="text/html; charset=utf-8")
diff --git a/python/ql/test/library-tests/web/django/views_2x_3x.py b/python/ql/test/library-tests/web/django/views_2x_3x.py
index 72371b789e9..d26b7915ef8 100644
--- a/python/ql/test/library-tests/web/django/views_2x_3x.py
+++ b/python/ql/test/library-tests/web/django/views_2x_3x.py
@@ -103,22 +103,26 @@ urlpatterns = [
 # FP reported in https://github.com/github/codeql-python-team/issues/38
 def fp_json_response(request):
     # implicitly sets Content-Type to "application/json"
-    return JsonResponse({"foo": request.GET.get("foo")}) # TODO
+    return JsonResponse({"foo": request.GET.get("foo")})
 
 # Not an XSS sink, since the Content-Type is not "text/html"
 def fp_manual_json_response(request):
     json_data = '{"json": "{}"}'.format(request.GET.get("foo"))
-    return HttpResponse(json_data, content_type="application/json") # TODO
+    return HttpResponse(json_data, content_type="application/json")
 
 # Not an XSS sink, since the Content-Type is not "text/html"
 def fp_manual_content_type(reuqest):
-    return HttpResponse('<img src="0" onerror="alert(1)">', content_type="text/plain") # TODO
+    return HttpResponse('<img src="0" onerror="alert(1)">', content_type="text/plain")
 
 # XSS FP reported in https://github.com/github/codeql/issues/3466
 # Note: This should be a open-redirect sink, but not a XSS sink.
 def fp_redirect(request):
     return HttpResponseRedirect(request.GET.get("next"))
 
-# Ensure that subclasses are still vuln to XSS
+# Ensure that simple subclasses are still vuln to XSS
 def tp_not_found(request):
     return HttpResponseNotFound(request.GET.get("name"))
+
+# Ensure we still have a XSS sink when manually setting the content_type to HTML
+def tp_manual_response_type(request):
+    return HttpResponse(request.GET.get("name"), content_type="text/html; charset=utf-8")
diff --git a/python/ql/test/query-tests/Security/lib/django/http/response.py b/python/ql/test/query-tests/Security/lib/django/http/response.py
index 96cc2bda9a9..1110a3cde19 100644
--- a/python/ql/test/query-tests/Security/lib/django/http/response.py
+++ b/python/ql/test/query-tests/Security/lib/django/http/response.py
@@ -1,13 +1,21 @@
 class HttpResponseBase(object):
     status_code = 200
 
+    def __init__(self, content_type=None, status=None, reason=None, charset=None):
+        pass
+
 
 class HttpResponse(HttpResponseBase):
-    pass
+    def __init__(self, content=b"", *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        # Content is a bytestring. See the `content` property methods.
+        self.content = content
 
 
 class HttpResponseRedirectBase(HttpResponse):
-    pass
+    def __init__(self, redirect_to, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        ...
 
 
 class HttpResponsePermanentRedirect(HttpResponseRedirectBase):

From 6cba2fe4f8d4ba674b5fbb44def20c7e5d97b920 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Tue, 26 May 2020 16:45:46 +0200
Subject: [PATCH 4/8] Python: Model Django response sinks that are not vuln to
 XSS

Since HttpResponse is not *only* used for XSS, it is still valuable to know the
content is send as part of the response.

The *proper* solution to this problem of not all HttpResponses being vulnerable
to XSS is probably to define a new abstract class in Http.qll called
HttpResponseXSSVulnerableSink (or similar). I would like to model a few more
libraries/frameworks before fully comitting to an approach though.
---
 .../ql/src/Security/CWE-079/ReflectedXss.ql   |  7 +-
 .../src/semmle/python/web/django/Redirect.qll |  2 +-
 .../src/semmle/python/web/django/Response.qll | 39 ++++++----
 .../src/semmle/python/web/django/Shared.qll   | 72 ++++++++++---------
 .../web/django/HttpResponseSinks.expected     |  4 ++
 5 files changed, 76 insertions(+), 48 deletions(-)

diff --git a/python/ql/src/Security/CWE-079/ReflectedXss.ql b/python/ql/src/Security/CWE-079/ReflectedXss.ql
index dbc02671603..cea41442c5b 100644
--- a/python/ql/src/Security/CWE-079/ReflectedXss.ql
+++ b/python/ql/src/Security/CWE-079/ReflectedXss.ql
@@ -28,7 +28,12 @@ class ReflectedXssConfiguration extends TaintTracking::Configuration {
         source instanceof HttpRequestTaintSource
     }
 
-    override predicate isSink(TaintTracking::Sink sink) { sink instanceof HttpResponseTaintSink }
+    override predicate isSink(TaintTracking::Sink sink) {
+        sink instanceof HttpResponseTaintSink and
+        not sink instanceof DjangoResponseContent
+        or
+        sink instanceof DjangoResponseContentXSSVulnerable
+    }
 }
 
 from ReflectedXssConfiguration config, TaintedPathSource src, TaintedPathSink sink
diff --git a/python/ql/src/semmle/python/web/django/Redirect.qll b/python/ql/src/semmle/python/web/django/Redirect.qll
index ca28a4a32d2..61ee041f904 100644
--- a/python/ql/src/semmle/python/web/django/Redirect.qll
+++ b/python/ql/src/semmle/python/web/django/Redirect.qll
@@ -27,7 +27,7 @@ class DjangoShortcutsRedirectSink extends HttpRedirectTaintSink {
 class DjangoRedirectResponseSink extends HttpRedirectTaintSink {
     DjangoRedirectResponseSink() {
         exists(CallNode call |
-            call = any(DjangoRedirectResponse rr).getACall()
+            call = any(DjangoRedirectResponseClass cls).getACall()
         |
             this = call.getArg(0)
             or
diff --git a/python/ql/src/semmle/python/web/django/Response.qll b/python/ql/src/semmle/python/web/django/Response.qll
index b2b2f2e162d..cd3b8dc5085 100644
--- a/python/ql/src/semmle/python/web/django/Response.qll
+++ b/python/ql/src/semmle/python/web/django/Response.qll
@@ -12,7 +12,7 @@ private class DjangoResponseKind extends TaintKind {
 /** INTENRAL taint-source used for tracking a django response. */
 private class DjangoResponseSource extends TaintSource {
     DjangoResponseSource() {
-        exists(DjangoXSSVulnerableResponse cls |
+        exists(DjangoContentResponseClass cls |
             cls.getACall() = this
         )
     }
@@ -37,21 +37,16 @@ class DjangoResponseWrite extends HttpResponseTaintSink {
     override string toString() { result = "django.Response.write(...)" }
 }
 
-/** An argument to initialization of a django response, which is vulnerable to external data (xss) */
+/**
+ * An argument to initialization of a django response.
+ */
 class DjangoResponseContent extends HttpResponseTaintSink {
+    DjangoContentResponseClass cls;
+    CallNode call;
+
     DjangoResponseContent() {
-        exists(CallNode call, DjangoXSSVulnerableResponse cls |
-            call = cls.getACall() and
-            this = cls.getContentArg(call) and
-            (
-                not exists(cls.getContentTypeArg(call))
-                or
-                exists(StringValue s |
-                    cls.getContentTypeArg(call).pointsTo(s) and
-                    s.getText().indexOf("text/html") = 0
-                )
-            )
-        )
+        call = cls.getACall() and
+        this = cls.getContentArg(call)
     }
 
     override predicate sinks(TaintKind kind) { kind instanceof StringKind }
@@ -59,6 +54,22 @@ class DjangoResponseContent extends HttpResponseTaintSink {
     override string toString() { result = "django.Response(...)" }
 }
 
+/**
+ * An argument to initialization of a django response, which is vulnerable to external data (XSS).
+ */
+class DjangoResponseContentXSSVulnerable extends DjangoResponseContent {
+    override DjangoXSSVulnerableResponseClass cls;
+
+    DjangoResponseContentXSSVulnerable() {
+        not exists(cls.getContentTypeArg(call))
+        or
+        exists(StringValue s |
+            cls.getContentTypeArg(call).pointsTo(s) and
+            s.getText().indexOf("text/html") = 0
+        )
+    }
+}
+
 class DjangoCookieSet extends CookieSet, CallNode {
     DjangoCookieSet() {
         any(DjangoResponseKind r).taints(this.getFunction().(AttrNode).getObject("set_cookie"))
diff --git a/python/ql/src/semmle/python/web/django/Shared.qll b/python/ql/src/semmle/python/web/django/Shared.qll
index b4402723273..3413cceb4ab 100644
--- a/python/ql/src/semmle/python/web/django/Shared.qll
+++ b/python/ql/src/semmle/python/web/django/Shared.qll
@@ -1,25 +1,8 @@
 import python
 
-/** A Class that is a Django Response (subclass of `django.http.HttpResponse`). */
-class DjangoResponse extends ClassValue {
-    ClassValue base;
-
-    DjangoResponse() {
-        (
-            // version 1.x
-            base = Value::named("django.http.response.HttpResponse")
-            or
-            // version 2.x and 3.x
-            // https://docs.djangoproject.com/en/2.2/ref/request-response/#httpresponse-objects
-            base = Value::named("django.http.HttpResponse")
-        ) and
-        this.getASuperType() = base
-    }
-}
-
-/** A Class that is a Django Redirect Response (subclass of `django.http.HttpResponseRedirectBase`). */
-class DjangoRedirectResponse extends DjangoResponse {
-    DjangoRedirectResponse() {
+/** A class that is a Django Redirect Response (subclass of `django.http.HttpResponseRedirectBase`). */
+class DjangoRedirectResponseClass extends ClassValue {
+    DjangoRedirectResponseClass() {
         exists(ClassValue redirect_base |
             // version 1.x
             redirect_base = Value::named("django.http.response.HttpResponseRedirectBase")
@@ -32,32 +15,57 @@ class DjangoRedirectResponse extends DjangoResponse {
     }
 }
 
+/**
+ * A class that is a Django Response, which can contain content.
+ * A subclass of `django.http.HttpResponse` that is not a `DjangoRedirectResponseClass`.
+ */
+class DjangoContentResponseClass extends ClassValue {
+    ClassValue base;
+
+    DjangoContentResponseClass() {
+        (
+            // version 1.x
+            base = Value::named("django.http.response.HttpResponse")
+            or
+            // version 2.x and 3.x
+            // https://docs.djangoproject.com/en/2.2/ref/request-response/#httpresponse-objects
+            base = Value::named("django.http.HttpResponse")
+        ) and
+        this.getASuperType() = base
+    }
+
+    // The reason these two method are defined in this class (and not in the Sink
+    // definition that uses this class), is that if we were to add support for
+    // `django.http.response.HttpResponseNotAllowed` it would make much more sense to add
+    // the custom logic in this class (or subclass), than to handle all of it in the sink
+    // definition.
+
+    /** Gets the `content` argument of a `call` to the constructor */
+    ControlFlowNode getContentArg(CallNode call) { none() }
+
+    /** Gets the `content_type` argument of a `call` to the constructor */
+    ControlFlowNode getContentTypeArg(CallNode call) { none() }
+}
+
 /** A Class that is a Django Response, and is vulnerable to XSS. */
-class DjangoXSSVulnerableResponse extends DjangoResponse {
-    DjangoXSSVulnerableResponse() {
+class DjangoXSSVulnerableResponseClass extends DjangoContentResponseClass{
+    DjangoXSSVulnerableResponseClass() {
         // We want to avoid FPs on subclasses that are not exposed to XSS, for example `JsonResponse`.
         // The easiest way is to disregard any subclass that has a special `__init__` method.
         // It's not guaranteed to remove all FPs, or not to generate FNs, but compared to our
         // previous implementation that would treat 0-th argument to _any_ subclass as a sink,
         // this gets us much closer to reality.
         this.lookup("__init__") = base.lookup("__init__") and
-        not this instanceof DjangoRedirectResponse
+        not this instanceof DjangoRedirectResponseClass
     }
 
-    // The reason these two method are defined in this class (and no in the Sink
-    // definition that uses this class), is that if we were to add support for `HttpResponseNotAllowed`
-    // it would make much more sense to add the custom logic in this class (or subclass), than to handle all of it
-    // in the sink definition.
-
-    /** Gets the `content` argument of a `call` to the constructor */
-    ControlFlowNode getContentArg(CallNode call) {
+    override ControlFlowNode getContentArg(CallNode call) {
         result = call.getArg(0)
         or
         result = call.getArgByName("content")
     }
 
-    /** Gets the `content_type` argument of a `call` to the constructor */
-    ControlFlowNode getContentTypeArg(CallNode call) {
+    override ControlFlowNode getContentTypeArg(CallNode call) {
         result = call.getArg(1)
         or
         result = call.getArgByName("content_type")
diff --git a/python/ql/test/library-tests/web/django/HttpResponseSinks.expected b/python/ql/test/library-tests/web/django/HttpResponseSinks.expected
index 7c9e583095f..e4e52c97514 100644
--- a/python/ql/test/library-tests/web/django/HttpResponseSinks.expected
+++ b/python/ql/test/library-tests/web/django/HttpResponseSinks.expected
@@ -8,6 +8,8 @@
 | views_1x.py:45:25:45:70 | django.Response(...) | externally controlled string |
 | views_1x.py:66:25:66:55 | django.Response(...) | externally controlled string |
 | views_1x.py:75:25:75:33 | django.Response(...) | externally controlled string |
+| views_1x.py:90:25:90:33 | django.Response(...) | externally controlled string |
+| views_1x.py:94:25:94:58 | django.Response(...) | externally controlled string |
 | views_1x.py:103:33:103:55 | django.Response(...) | externally controlled string |
 | views_1x.py:107:25:107:47 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:8:25:8:63 | django.Response(...) | externally controlled string |
@@ -23,5 +25,7 @@
 | views_2x_3x.py:82:25:82:69 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:85:25:85:64 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:88:25:88:32 | django.Response(...) | externally controlled string |
+| views_2x_3x.py:111:25:111:33 | django.Response(...) | externally controlled string |
+| views_2x_3x.py:115:25:115:58 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:124:33:124:55 | django.Response(...) | externally controlled string |
 | views_2x_3x.py:128:25:128:47 | django.Response(...) | externally controlled string |

From daa1b6fc790e5759385917404b377e4c33f1d0c6 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Mon, 22 Jun 2020 13:41:03 +0200
Subject: [PATCH 5/8] Python: Fix grammar in QLDoc

Co-authored-by: Taus <tausbn@gmail.com>
---
 python/ql/src/semmle/python/web/django/Response.qll | 2 +-
 python/ql/src/semmle/python/web/django/Shared.qll   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/python/ql/src/semmle/python/web/django/Response.qll b/python/ql/src/semmle/python/web/django/Response.qll
index cd3b8dc5085..a0e07ea4b21 100644
--- a/python/ql/src/semmle/python/web/django/Response.qll
+++ b/python/ql/src/semmle/python/web/django/Response.qll
@@ -9,7 +9,7 @@ private class DjangoResponseKind extends TaintKind {
     DjangoResponseKind() { this = "django.response.HttpResponse" }
 }
 
-/** INTENRAL taint-source used for tracking a django response. */
+/** INTERNAL taint-source used for tracking a django response object. */
 private class DjangoResponseSource extends TaintSource {
     DjangoResponseSource() {
         exists(DjangoContentResponseClass cls |
diff --git a/python/ql/src/semmle/python/web/django/Shared.qll b/python/ql/src/semmle/python/web/django/Shared.qll
index 3413cceb4ab..f66acc7fe2d 100644
--- a/python/ql/src/semmle/python/web/django/Shared.qll
+++ b/python/ql/src/semmle/python/web/django/Shared.qll
@@ -34,7 +34,7 @@ class DjangoContentResponseClass extends ClassValue {
         this.getASuperType() = base
     }
 
-    // The reason these two method are defined in this class (and not in the Sink
+    // The reason these two methods are defined in this class (and not in the Sink
     // definition that uses this class), is that if we were to add support for
     // `django.http.response.HttpResponseNotAllowed` it would make much more sense to add
     // the custom logic in this class (or subclass), than to handle all of it in the sink
@@ -47,7 +47,7 @@ class DjangoContentResponseClass extends ClassValue {
     ControlFlowNode getContentTypeArg(CallNode call) { none() }
 }
 
-/** A Class that is a Django Response, and is vulnerable to XSS. */
+/** A class that is a Django Response, and is vulnerable to XSS. */
 class DjangoXSSVulnerableResponseClass extends DjangoContentResponseClass{
     DjangoXSSVulnerableResponseClass() {
         // We want to avoid FPs on subclasses that are not exposed to XSS, for example `JsonResponse`.

From 4a7bfbe0918ddf2cb35b91e034a0ce57131933fc Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Thu, 2 Jul 2020 11:43:23 +0200
Subject: [PATCH 6/8] Python: Use .matches instead of .indexOf() = 0

---
 python/ql/src/semmle/python/web/django/Response.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/ql/src/semmle/python/web/django/Response.qll b/python/ql/src/semmle/python/web/django/Response.qll
index a0e07ea4b21..5ceef4516f5 100644
--- a/python/ql/src/semmle/python/web/django/Response.qll
+++ b/python/ql/src/semmle/python/web/django/Response.qll
@@ -65,7 +65,7 @@ class DjangoResponseContentXSSVulnerable extends DjangoResponseContent {
         or
         exists(StringValue s |
             cls.getContentTypeArg(call).pointsTo(s) and
-            s.getText().indexOf("text/html") = 0
+            s.getText().matches("text/html%")
         )
     }
 }

From a947d151e5130c37a386a5333de4b74d2d5ab7d8 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Thu, 2 Jul 2020 11:53:25 +0200
Subject: [PATCH 7/8] Python: Django changes now backwards compatible
 deprecation

---
 python/ql/src/semmle/python/web/django/Redirect.qll |  3 +++
 python/ql/src/semmle/python/web/django/Response.qll | 10 ++++++++++
 python/ql/src/semmle/python/web/django/Shared.qll   | 12 ++++++++++++
 3 files changed, 25 insertions(+)

diff --git a/python/ql/src/semmle/python/web/django/Redirect.qll b/python/ql/src/semmle/python/web/django/Redirect.qll
index 61ee041f904..b3b52f5e03e 100644
--- a/python/ql/src/semmle/python/web/django/Redirect.qll
+++ b/python/ql/src/semmle/python/web/django/Redirect.qll
@@ -21,6 +21,9 @@ class DjangoShortcutsRedirectSink extends HttpRedirectTaintSink {
     }
 }
 
+/** DEPRECATED: Use `DjangoShortcutsRedirectSink` instead. */
+deprecated class DjangoRedirect = DjangoShortcutsRedirectSink;
+
 /**
  * The URL argument when instantiating a Django Redirect Response.
  */
diff --git a/python/ql/src/semmle/python/web/django/Response.qll b/python/ql/src/semmle/python/web/django/Response.qll
index 5ceef4516f5..df27d319492 100644
--- a/python/ql/src/semmle/python/web/django/Response.qll
+++ b/python/ql/src/semmle/python/web/django/Response.qll
@@ -4,6 +4,16 @@ import semmle.python.security.strings.Basic
 private import semmle.python.web.django.Shared
 private import semmle.python.web.Http
 
+/**
+ * DEPRECATED: This class is internal to the django library modeling, and should
+ * never be used by anyone.
+ *
+ * A django.http.response.Response object
+ * This isn't really a "taint", but we use the value tracking machinery to
+ * track the flow of response objects.
+ */
+deprecated class DjangoResponse = DjangoResponseKind;
+
 /** INTERNAL class used for tracking a django response object. */
 private class DjangoResponseKind extends TaintKind {
     DjangoResponseKind() { this = "django.response.HttpResponse" }
diff --git a/python/ql/src/semmle/python/web/django/Shared.qll b/python/ql/src/semmle/python/web/django/Shared.qll
index f66acc7fe2d..b98bd803fcf 100644
--- a/python/ql/src/semmle/python/web/django/Shared.qll
+++ b/python/ql/src/semmle/python/web/django/Shared.qll
@@ -1,5 +1,17 @@
 import python
 
+/** DEPRECATED: Use `Value::named("django.shortcuts.redirect")` instead. */
+deprecated FunctionValue redirect() { result = Value::named("django.shortcuts.redirect") }
+
+/** DEPRECATED: Use `DjangoRedirectResponseClass` instead. */
+deprecated ClassValue theDjangoHttpRedirectClass() {
+    // version 1.x
+    result = Value::named("django.http.response.HttpResponseRedirectBase")
+    or
+    // version 2.x
+    result = Value::named("django.http.HttpResponseRedirectBase")
+}
+
 /** A class that is a Django Redirect Response (subclass of `django.http.HttpResponseRedirectBase`). */
 class DjangoRedirectResponseClass extends ClassValue {
     DjangoRedirectResponseClass() {

From 9a829271877efd74a5f7851c0ece3f692b905fe9 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Thu, 2 Jul 2020 11:54:41 +0200
Subject: [PATCH 8/8] Python: Autoformat

---
 python/ql/src/semmle/python/web/django/Redirect.qll | 4 +---
 python/ql/src/semmle/python/web/django/Response.qll | 6 +-----
 python/ql/src/semmle/python/web/django/Shared.qll   | 3 +--
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/python/ql/src/semmle/python/web/django/Redirect.qll b/python/ql/src/semmle/python/web/django/Redirect.qll
index b3b52f5e03e..ad46d985b8c 100644
--- a/python/ql/src/semmle/python/web/django/Redirect.qll
+++ b/python/ql/src/semmle/python/web/django/Redirect.qll
@@ -29,9 +29,7 @@ deprecated class DjangoRedirect = DjangoShortcutsRedirectSink;
  */
 class DjangoRedirectResponseSink extends HttpRedirectTaintSink {
     DjangoRedirectResponseSink() {
-        exists(CallNode call |
-            call = any(DjangoRedirectResponseClass cls).getACall()
-        |
+        exists(CallNode call | call = any(DjangoRedirectResponseClass cls).getACall() |
             this = call.getArg(0)
             or
             this = call.getArgByName("redirect_to")
diff --git a/python/ql/src/semmle/python/web/django/Response.qll b/python/ql/src/semmle/python/web/django/Response.qll
index df27d319492..a44af076213 100644
--- a/python/ql/src/semmle/python/web/django/Response.qll
+++ b/python/ql/src/semmle/python/web/django/Response.qll
@@ -21,11 +21,7 @@ private class DjangoResponseKind extends TaintKind {
 
 /** INTERNAL taint-source used for tracking a django response object. */
 private class DjangoResponseSource extends TaintSource {
-    DjangoResponseSource() {
-        exists(DjangoContentResponseClass cls |
-            cls.getACall() = this
-        )
-    }
+    DjangoResponseSource() { exists(DjangoContentResponseClass cls | cls.getACall() = this) }
 
     override predicate isSourceOf(TaintKind kind) { kind instanceof DjangoResponseKind }
 
diff --git a/python/ql/src/semmle/python/web/django/Shared.qll b/python/ql/src/semmle/python/web/django/Shared.qll
index b98bd803fcf..ea856ddf65a 100644
--- a/python/ql/src/semmle/python/web/django/Shared.qll
+++ b/python/ql/src/semmle/python/web/django/Shared.qll
@@ -51,7 +51,6 @@ class DjangoContentResponseClass extends ClassValue {
     // `django.http.response.HttpResponseNotAllowed` it would make much more sense to add
     // the custom logic in this class (or subclass), than to handle all of it in the sink
     // definition.
-
     /** Gets the `content` argument of a `call` to the constructor */
     ControlFlowNode getContentArg(CallNode call) { none() }
 
@@ -60,7 +59,7 @@ class DjangoContentResponseClass extends ClassValue {
 }
 
 /** A class that is a Django Response, and is vulnerable to XSS. */
-class DjangoXSSVulnerableResponseClass extends DjangoContentResponseClass{
+class DjangoXSSVulnerableResponseClass extends DjangoContentResponseClass {
     DjangoXSSVulnerableResponseClass() {
         // We want to avoid FPs on subclasses that are not exposed to XSS, for example `JsonResponse`.
         // The easiest way is to disregard any subclass that has a special `__init__` method.