Skip to content

Commit 72ea4ff

Browse files
committed
Python: Add more tests of django responses
They clearly shouldn't all be XSS sinks
1 parent 14664be commit 72ea4ff

File tree

5 files changed

+93
-21
lines changed

5 files changed

+93
-21
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@
88
| views_1x.py:45:25:45:70 | django.Response(...) | externally controlled string |
99
| views_1x.py:66:25:66:55 | django.Response(...) | externally controlled string |
1010
| views_1x.py:75:25:75:33 | django.Response(...) | externally controlled string |
11+
| views_1x.py:85:25:85:55 | django.Response(...) | externally controlled string |
12+
| views_1x.py:90:25:90:33 | django.Response(...) | externally controlled string |
13+
| views_1x.py:94:25:94:58 | django.Response(...) | externally controlled string |
14+
| views_1x.py:99:33:99:55 | django.Response(...) | externally controlled string |
15+
| views_1x.py:103:33:103:55 | django.Response(...) | externally controlled string |
1116
| views_2x_3x.py:8:25:8:63 | django.Response(...) | externally controlled string |
1217
| views_2x_3x.py:12:25:12:52 | django.Response(...) | externally controlled string |
1318
| views_2x_3x.py:16:25:16:53 | django.Response(...) | externally controlled string |
@@ -21,3 +26,8 @@
2126
| views_2x_3x.py:82:25:82:69 | django.Response(...) | externally controlled string |
2227
| views_2x_3x.py:85:25:85:64 | django.Response(...) | externally controlled string |
2328
| views_2x_3x.py:88:25:88:32 | django.Response(...) | externally controlled string |
29+
| views_2x_3x.py:106:25:106:55 | django.Response(...) | externally controlled string |
30+
| views_2x_3x.py:111:25:111:33 | django.Response(...) | externally controlled string |
31+
| views_2x_3x.py:115:25:115:58 | django.Response(...) | externally controlled string |
32+
| views_2x_3x.py:120:33:120:55 | django.Response(...) | externally controlled string |
33+
| views_2x_3x.py:124:33:124:55 | django.Response(...) | externally controlled string |

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"""test of views for Django 1.x"""
22
from django.conf.urls import patterns, url
3-
from django.http.response import HttpResponse
3+
from django.http.response import HttpResponse, HttpResponseRedirect, JsonResponse, HttpResponseNotFound
44
from django.views.generic import View
55

66

@@ -77,3 +77,27 @@ def kw_args(request):
7777
urlpatterns = [
7878
url(view=kw_args, regex=r'^kw_args$')
7979
]
80+
81+
# Not an XSS sink, since the Content-Type is not "text/html"
82+
# FP reported in https://github.com/github/codeql-python-team/issues/38
83+
def fp_json_response(request):
84+
# implicitly sets Content-Type to "application/json"
85+
return JsonResponse({"foo": request.GET.get("foo")})
86+
87+
# Not an XSS sink, since the Content-Type is not "text/html"
88+
def fp_manual_json_response(request):
89+
json_data = '{"json": "{}"}'.format(request.GET.get("foo"))
90+
return HttpResponse(json_data, content_type="application/json")
91+
92+
# Not an XSS sink, since the Content-Type is not "text/html"
93+
def fp_manual_content_type(reuqest):
94+
return HttpResponse('<img src="0" onerror="alert(1)">', content_type="text/plain")
95+
96+
# XSS FP reported in https://github.com/github/codeql/issues/3466
97+
# Note: This should be a open-redirect sink, but not a XSS sink.
98+
def fp_redirect(request):
99+
return HttpResponseRedirect(request.GET.get("next"))
100+
101+
# Ensure that subclasses are still vuln to XSS
102+
def tp_not_found(request):
103+
return HttpResponseNotFound(request.GET.get("name"))

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

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"""testing views for Django 2.x and 3.x"""
22
from django.urls import path, re_path
3-
from django.http import HttpResponse
3+
from django.http import HttpResponse, HttpResponseRedirect, JsonResponse, HttpResponseNotFound
44
from django.views import View
55

66

@@ -99,24 +99,26 @@ def not_valid_identifier(request):
9999
]
100100

101101

102-
################################################################################
103-
102+
# Not an XSS sink, since the Content-Type is not "text/html"
103+
# FP reported in https://github.com/github/codeql-python-team/issues/38
104+
def fp_json_response(request):
105+
# implicitly sets Content-Type to "application/json"
106+
return JsonResponse({"foo": request.GET.get("foo")}) # TODO
104107

105-
# We should abort if a decorator is used. As demonstrated below, anything might happen
108+
# Not an XSS sink, since the Content-Type is not "text/html"
109+
def fp_manual_json_response(request):
110+
json_data = '{"json": "{}"}'.format(request.GET.get("foo"))
111+
return HttpResponse(json_data, content_type="application/json") # TODO
106112

107-
# def reverse_kwargs(f):
108-
# @wraps(f)
109-
# def f_(*args, **kwargs):
110-
# new_kwargs = dict()
111-
# for key, value in kwargs.items():
112-
# new_kwargs[key[::-1]] = value
113-
# return f(*args, **new_kwargs)
114-
# return f_
113+
# Not an XSS sink, since the Content-Type is not "text/html"
114+
def fp_manual_content_type(reuqest):
115+
return HttpResponse('<img src="0" onerror="alert(1)">', content_type="text/plain") # TODO
115116

116-
# @reverse_kwargs
117-
# def decorators_can_do_anything(request, oof, foo=None):
118-
# return HttpResponse('This is a mess'[::-1])
117+
# XSS FP reported in https://github.com/github/codeql/issues/3466
118+
# Note: This should be a open-redirect sink, but not a XSS sink.
119+
def fp_redirect(request):
120+
return HttpResponseRedirect(request.GET.get("next")) # TODO
119121

120-
# urlpatterns = [
121-
# path('rev/<foo>', decorators_can_do_anything),
122-
# ]
122+
# Ensure that subclasses are still vuln to XSS
123+
def tp_not_found(request):
124+
return HttpResponseNotFound(request.GET.get("name"))
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
from .response import HttpResponse
1+
from .response import *
22
from .request import HttpRequest
Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,38 @@
1-
class HttpResponse(object):
1+
class HttpResponseBase(object):
2+
status_code = 200
3+
4+
5+
class HttpResponse(HttpResponseBase):
26
pass
7+
8+
9+
class HttpResponseRedirectBase(HttpResponse):
10+
pass
11+
12+
13+
class HttpResponsePermanentRedirect(HttpResponseRedirectBase):
14+
status_code = 301
15+
16+
17+
class HttpResponseRedirect(HttpResponseRedirectBase):
18+
status_code = 302
19+
20+
21+
class HttpResponseNotFound(HttpResponse):
22+
status_code = 404
23+
24+
25+
class JsonResponse(HttpResponse):
26+
27+
def __init__(
28+
self,
29+
data,
30+
encoder=...,
31+
safe=True,
32+
json_dumps_params=None,
33+
**kwargs
34+
):
35+
# fake code to represent what is going on :)
36+
kwargs.setdefault("content_type", "application/json")
37+
data = str(data)
38+
super().__init__(content=data, **kwargs)

0 commit comments

Comments
 (0)