Merge branch 'master' into python-keyword-only-args

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 |

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

@@ -8,6 +8,10 @@
 | 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 |
 | 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 +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 |

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,31 @@ 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 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")

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,30 @@ 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")})
 
+# 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")
 
-# 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")
 
-# 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"))
 
-# @reverse_kwargs
-# def decorators_can_do_anything(request, oof, foo=None):
-#     return HttpResponse('This is a mess'[::-1])
+# Ensure that simple subclasses are still vuln to XSS
+def tp_not_found(request):
+    return HttpResponseNotFound(request.GET.get("name"))
 
-# urlpatterns = [
-#     path('rev/<foo>', decorators_can_do_anything),
-# ]
+# 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")

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

python/ql/test/query-tests/Security/lib/django/http/response.py

@@ -1,2 +1,46 @@
-class HttpResponse(object):
-    pass
+class HttpResponseBase(object):
+    status_code = 200
+
+    def __init__(self, content_type=None, status=None, reason=None, charset=None):
+        pass
+
+
+class HttpResponse(HttpResponseBase):
+    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):
+    def __init__(self, redirect_to, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        ...
+
+
+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)