Python: Consolidate tests for django

The tests in 3/ was not Python 3 specific anymore

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

@@ -1,13 +1,15 @@
-| test.py:14 | Str | externally controlled string |
-| test.py:15 | Str | externally controlled string |
-| test.py:18 | BinaryExpr | externally controlled string |
-| test.py:21 | BinaryExpr | externally controlled string |
-| test.py:22 | BinaryExpr | externally controlled string |
-| test.py:23 | BinaryExpr | externally controlled string |
-| test.py:37 | Str | externally controlled string |
-| test.py:44 | BinaryExpr | externally controlled string |
-| test.py:48 | Attribute() | externally controlled string |
-| test.py:59 | Attribute() | externally controlled string |
-| test.py:70 | Attribute() | externally controlled string |
-| test.py:73 | Attribute() | externally controlled string |
-| test.py:76 | Attribute() | externally controlled string |
+| sql.py:13 | Str | externally controlled string |
+| sql.py:14 | Str | externally controlled string |
+| sql.py:17 | BinaryExpr | externally controlled string |
+| sql.py:20 | BinaryExpr | externally controlled string |
+| sql.py:21 | BinaryExpr | externally controlled string |
+| sql.py:22 | BinaryExpr | externally controlled string |
+| sql.py:36 | Str | externally controlled string |
+| sql.py:42 | BinaryExpr | externally controlled string |
+| sql.py:47 | BinaryExpr | externally controlled string |
+| views.py:7 | Attribute() | externally controlled string |
+| views.py:11 | Attribute() | externally controlled string |
+| views.py:15 | Attribute() | externally controlled string |
+| views.py:22 | Attribute() | externally controlled string |
+| views.py:27 | Attribute() | externally controlled string |
+| views.py:31 | Attribute() | externally controlled string |

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

@@ -1,17 +1,17 @@
+| test.py:5 | path | externally controlled string |
+| test.py:5 | request | django.request.HttpRequest |
+| test.py:11 | path | externally controlled string |
 | test.py:11 | request | django.request.HttpRequest |
-| test.py:11 | username | externally controlled string |
-| test.py:41 | request | django.request.HttpRequest |
-| test.py:47 | bar | externally controlled string |
-| test.py:47 | foo | externally controlled string |
-| test.py:47 | request | django.request.HttpRequest |
-| test.py:58 | page_number | externally controlled string |
-| test.py:58 | request | django.request.HttpRequest |
-| test.py:69 | arg0 | externally controlled string |
-| test.py:69 | request | django.request.HttpRequest |
-| test.py:72 | arg0 | externally controlled string |
-| test.py:72 | arg1 | externally controlled string |
-| test.py:72 | arg2 | externally controlled string |
-| test.py:72 | request | django.request.HttpRequest |
-| test.py:75 | arg0 | externally controlled string |
-| test.py:75 | arg1 | externally controlled string |
-| test.py:75 | request | django.request.HttpRequest |
+| views.py:6 | bar | externally controlled string |
+| views.py:6 | foo | externally controlled string |
+| views.py:6 | request | django.request.HttpRequest |
+| views.py:10 | request | django.request.HttpRequest |
+| views.py:14 | request | django.request.HttpRequest |
+| views.py:25 | page_number | externally controlled string |
+| views.py:25 | request | django.request.HttpRequest |
+| views.py:30 | arg0 | externally controlled string |
+| views.py:30 | arg1 | externally controlled string |
+| views.py:30 | request | django.request.HttpRequest |
+| views.py:50 | request | django.request.HttpRequest |
+| views.py:50 | username | externally controlled string |
+| views.py:59 | request | django.request.HttpRequest |

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

@@ -0,0 +1,53 @@
+from django.db import connection, models
+from django.db.models.expressions import RawSQL
+
+
+class User(models.Model):
+    username = models.CharField(max_length=100)
+    description = models.TextField(blank=True)
+
+
+def show_user(username):
+    with connection.cursor() as cursor:
+        # GOOD -- Using parameters
+        cursor.execute("SELECT * FROM users WHERE username = %s", username)
+        User.objects.raw("SELECT * FROM users WHERE username = %s", (username,))
+
+        # BAD -- Using string formatting
+        cursor.execute("SELECT * FROM users WHERE username = '%s'" % username)
+
+        # BAD -- other ways of executing raw SQL code with string interpolation
+        User.objects.annotate(RawSQL("insert into names_file ('name') values ('%s')" % username))
+        User.objects.raw("insert into names_file ('name') values ('%s')" % username)
+        User.objects.extra("insert into names_file ('name') values ('%s')" % username)
+
+        # BAD (but currently no custom query to find this)
+        #
+        # It is exposed to SQL injection (https://docs.djangoproject.com/en/2.2/ref/models/querysets/#extra)
+        # For example, using name = "; DROP ALL TABLES -- "
+        # will result in SQL: SELECT * FROM name WHERE name = ''; DROP ALL TABLES -- ''
+        #
+        # This shouldn't be very widespread, since using a normal string will result in invalid SQL
+        # Using name = "example", will result in SQL: SELECT * FROM name WHERE name = ''example''
+        # which in MySQL will give a syntax error
+        #
+        # When testing this out locally, none of the queries worked against SQLite3, but I could use
+        # the SQL injection against MySQL.
+        User.objects.raw("SELECT * FROM users WHERE username = '%s'", (username,))
+
+
+def raw3(arg):
+    m = User.objects.filter('foo')
+    m = m.filter('bar')
+    m.raw("select foo from bar where baz = %s" % arg)
+
+
+def raw4(arg):
+    m = User.objects.filter('foo')
+    m.extra("select foo from bar where baz = %s" % arg)
+
+
+def update_user(key, description1):
+    # Neither of these are exposed to sql-injections
+    user = User.objects.get(pk=key)
+    item.description = description

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

@@ -1,84 +1,18 @@
+from django.conf.urls import url
+from django.shortcuts import redirect, render
 
-from django.conf.urls import patterns, url
-from django.db import connection, models
-from django.db.models.expressions import RawSQL
-from django.http.response import HttpResponse
-import base64
 
-class User(models.Model):
-    pass
+def with_template(request, path='default'):
+    env = {'path': path}
+    # We would need to understand django templates to know if this is safe or not
+    return render(request, 'possibly-vulnerable-template.html', env)
 
-def show_user(request, username):
-    with connection.cursor() as cursor:
-        # GOOD -- Using parameters
-        cursor.execute("SELECT * FROM users WHERE username = %s", username)
-        User.objects.raw("SELECT * FROM users WHERE username = %s", (username,))
 
-        # BAD -- Using string formatting
-        cursor.execute("SELECT * FROM users WHERE username = '%s'" % username)
+def vuln_redirect(request, path):
+    return redirect(path)
 
-        # BAD -- other ways of executing raw SQL code with string interpolation
-        User.objects.annotate(RawSQL("insert into names_file ('name') values ('%s')" % username))
-        User.objects.raw("insert into names_file ('name') values ('%s')" % username)
-        User.objects.extra("insert into names_file ('name') values ('%s')" % username)
-
-        # BAD (but currently no custom query to find this)
-        #
-        # It is exposed to SQL injection (https://docs.djangoproject.com/en/2.2/ref/models/querysets/#extra)
-        # For example, using name = "; DROP ALL TABLES -- "
-        # will result in SQL: SELECT * FROM name WHERE name = ''; DROP ALL TABLES -- ''
-        #
-        # This shouldn't be very widespread, since using a normal string will result in invalid SQL
-        # Using name = "example", will result in SQL: SELECT * FROM name WHERE name = ''example''
-        # which in MySQL will give a syntax error
-        #
-        # When testing this out locally, none of the queries worked against SQLite3, but I could use
-        # the SQL injection against MySQL.
-        User.objects.raw("SELECT * FROM users WHERE username = '%s'", (username,))
-
-urlpatterns = patterns(url(r'^users/(?P<username>[^/]+)$', show_user))
-
-def maybe_xss(request):
-    first_name = request.POST.get('first_name', '')
-    resp = HttpResponse()
-    resp.write("first name is " + first_name)
-    return resp
-
-def xss_kwargs(request, foo, bar, baz=None):
-    return HttpResponse('xss_kwargs: {} {}'.format(foo, bar))
 
 urlpatterns = [
-    url(r'^maybe_xss$', maybe_xss, name='maybe_xss'),
-    url(r'^bar/(?P<bar>[^/]+)/foo/(?P<foo>[^/]+)', xss_kwargs, name='xss_kwargs'),
-]
-
-
-# Non capturing group (we correctly identify page_number as a request parameter)
-
-def show_articles(request, page_number=1):
-    return HttpResponse('articles page: {}'.format(page_number))
-
-urlpatterns = [
-    # one pattern to support `articles/page-<n>` and ensuring that articles/ goes to page-1
-    url(r'articles/^(?:page-(?P<page_number>\d+)/)?$', show_articles),
-]
-
-
-# Positional arguments
-
-def xxs_positional_arg1(request, arg0):
-    return HttpResponse('xxs_positional_arg1: {}'.format(arg0))
-
-def xxs_positional_arg2(request, arg0, arg1, arg2):
-    return HttpResponse('xxs_positional_arg2: {} {} {}'.format(arg0, arg1, arg2))
-
-def xxs_positional_arg3(request, arg0, arg1):
-    return HttpResponse('xxs_positional_arg3: {} {}'.format(arg0, arg1))
-
-urlpatterns = [
-    # passing as positional argument is not the recommended way of doing things,
-    # but it is certainly possible
-    url(r'^(.+)$', xxs_positional_arg1, name='xxs_positional_arg1'),
-    url(r'^([^/]+)/([^/]+)/([^/]+)$', xxs_positional_arg2, name='xxs_positional_arg2'),
-    url(r'^([^/]+)/(?:foo|bar)/([^/]+)$', xxs_positional_arg3, name='xxs_positional_arg3'),
+    url(r'^(?P<path>.*)$', with_template),
+    url(r'^redirect/(?P<path>.*)$', vuln_redirect),
 ]

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

@@ -0,0 +1,65 @@
+from django.conf.urls import patterns, url
+from django.http.response import HttpResponse
+from django.views.generic import View
+
+
+def url_match_xss(request, foo, bar, no_taint=None):
+    return HttpResponse('url_match_xss: {} {}'.format(foo, bar))
+
+
+def get_params_xss(request):
+    return HttpResponse(request.GET.get("untrusted"))
+
+
+def post_params_xss(request):
+    return HttpResponse(request.POST.get("untrusted"))
+
+
+class ClassView(View):
+
+    # TODO: Currently we don't flag `untrusted` as a DjangoRequestParameter
+    def get(self, request, untrusted):
+        return HttpResponse('ClassView: {}'.format(untrusted))
+
+
+def show_articles(request, page_number=1):
+    page_number = int(page_number)
+    return HttpResponse('articles page: {}'.format(page_number))
+
+
+def xxs_positional_arg(request, arg0, arg1, no_taint=None):
+    return HttpResponse('xxs_positional_arg: {} {}'.format(arg0, arg1))
+
+
+urlpatterns = [
+    url(r'^url_match/(?P<foo>[^/]+)/(?P<bar>[^/]+)$', url_match_xss),
+    url(r'^get_params$', get_params_xss),
+    url(r'^post_params$', post_params_xss),
+    url(r'^class_view/(?P<untrusted>.+)$', ClassView.as_view()),
+
+    # one pattern to support `articles/page-<n>` and ensuring that articles/ goes to page-1
+    url(r'articles/^(?:page-(?P<page_number>\d+)/)?$', show_articles),
+    # passing as positional argument is not the recommended way of doing things, but it is certainly
+    # possible
+    url(r'^([^/]+)/(?:foo|bar)/([^/]+)$', xxs_positional_arg, name='xxs_positional_arg'),
+]
+
+
+# Using patterns() for routing
+
+def show_user(request, username):
+    pass
+
+
+urlpatterns = patterns(url(r'^users/(?P<username>[^/]+)$', show_user))
+
+
+# Show we understand the keyword arguments to django.conf.urls.url
+
+def we_understand_url_kwargs(request):
+    pass
+
+
+urlpatterns = [
+    url(view=we_understand_url_kwargs, regex=r'^specifying-as-kwargs-is-not-a-problem$')
+]