hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-04 13:15:21 +02:00
Code Issues Packages Projects Releases Wiki Activity

Python: Don't quote %s in django example

Browse Source 
This is vulnerable to SQL injection because of the quotes around %s -- added
some code that highlights this in test.py

Since our examples did this in the safe query, I ended up rewriting them
completely, causing a lot of trouble for myself :D
This commit is contained in:
Rasmus Wriedt Larsen
2019-10-14 16:51:50 +02:00
parent afe7a0536c
commit 471318369b
7 changed files with 128 additions and 90 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -1,10 +1,13 @@
| test.py:18 | Str | externally controlled string |
| 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:24 | BinaryExpr | externally controlled string |
| test.py:25 | BinaryExpr | externally controlled string |
| test.py:26 | BinaryExpr | externally controlled string |
| test.py:34 | BinaryExpr | externally controlled string |
| test.py:46 | Attribute() | externally controlled string |
| test.py:57 | Attribute() | externally controlled string |
| test.py:60 | Attribute() | externally controlled string |
| test.py:63 | Attribute() | 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 |

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

@@ -1,11 +1,17 @@
| test.py:11 | request | django.request.HttpRequest |
| test.py:31 | request | django.request.HttpRequest |
| test.py:56 | arg0 | externally controlled string |
| test.py:56 | request | django.request.HttpRequest |
| test.py:59 | arg0 | externally controlled string |
| test.py:59 | arg1 | externally controlled string |
| test.py:59 | arg2 | externally controlled string |
| test.py:59 | request | django.request.HttpRequest |
| test.py:62 | arg0 | externally controlled string |
| test.py:62 | arg1 | externally controlled string |
| test.py:62 | 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 |

51
python/ql/test/library-tests/web/django/test.py
View File

@@ -5,28 +5,38 @@ from django.db.models.expressions import RawSQL
from django.http.response import HttpResponse
import base64
class Name(models.Model):
class User(models.Model):
pass
def save_name(request):
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,))
if request.method == 'POST':
name = request.POST.get('name')
curs = connection.cursor()
#GOOD -- Using parameters
curs.execute(
"insert into names_file ('name') values ('%s')", name)
#BAD -- Using string formatting
curs.execute(
"insert into names_file ('name') values ('%s')" % name)
# BAD -- Using string formatting
cursor.execute("SELECT * FROM users WHERE username = '%s'" % username)
#BAD -- other ways of executing raw SQL code with string interpolation
Name.objects.annotate(RawSQL("insert into names_file ('name') values ('%s')" % name))
Name.objects.raw("insert into names_file ('name') values ('%s')" % name)
Name.objects.extra("insert into names_file ('name') values ('%s')" % name)
# 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)
urlpatterns1 = patterns(url(r'^save_name/$',
save_name, name='save_name'))
# 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', '')
@@ -34,9 +44,12 @@ def maybe_xss(request):
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 = [
# Route to code_execution
url(r'^maybe_xss$', maybe_xss, name='maybe_xss')
url(r'^maybe_xss$', maybe_xss, name='maybe_xss'),
url(r'^bar/(?P<bar>[^/]+)/foo/(?P<foo>[^/]+)', xss_kwargs, name='xss_kwargs'),
]

27
python/ql/test/query-tests/Security/CWE-089/SqlInjection.expected
View File

@@ -1,17 +1,14 @@
edges
| sql_injection.py:9:15:9:21 | django.request.HttpRequest | sql_injection.py:12:16:12:22 | django.request.HttpRequest |
| sql_injection.py:12:16:12:22 | django.request.HttpRequest | sql_injection.py:12:16:12:27 | django.http.request.QueryDict |
| sql_injection.py:12:16:12:27 | django.http.request.QueryDict | sql_injection.py:12:16:12:39 | externally controlled string |
| sql_injection.py:12:16:12:39 | externally controlled string | sql_injection.py:19:63:19:66 | externally controlled string |
| sql_injection.py:12:16:12:39 | externally controlled string | sql_injection.py:22:88:22:91 | externally controlled string |
| sql_injection.py:12:16:12:39 | externally controlled string | sql_injection.py:23:76:23:79 | externally controlled string |
| sql_injection.py:12:16:12:39 | externally controlled string | sql_injection.py:24:78:24:81 | externally controlled string |
| sql_injection.py:19:63:19:66 | externally controlled string | sql_injection.py:19:13:19:66 | externally controlled string |
| sql_injection.py:22:88:22:91 | externally controlled string | sql_injection.py:22:38:22:91 | externally controlled string |
| sql_injection.py:23:76:23:79 | externally controlled string | sql_injection.py:23:26:23:79 | externally controlled string |
| sql_injection.py:24:78:24:81 | externally controlled string | sql_injection.py:24:28:24:81 | externally controlled string |
| sql_injection.py:12:24:12:31 | externally controlled string | sql_injection.py:19:70:19:77 | externally controlled string |
| sql_injection.py:12:24:12:31 | externally controlled string | sql_injection.py:22:88:22:95 | externally controlled string |
| sql_injection.py:12:24:12:31 | externally controlled string | sql_injection.py:23:76:23:83 | externally controlled string |
| sql_injection.py:12:24:12:31 | externally controlled string | sql_injection.py:24:78:24:85 | externally controlled string |
| sql_injection.py:19:70:19:77 | externally controlled string | sql_injection.py:19:24:19:77 | externally controlled string |
| sql_injection.py:22:88:22:95 | externally controlled string | sql_injection.py:22:38:22:95 | externally controlled string |
| sql_injection.py:23:76:23:83 | externally controlled string | sql_injection.py:23:26:23:83 | externally controlled string |
| sql_injection.py:24:78:24:85 | externally controlled string | sql_injection.py:24:28:24:85 | externally controlled string |
#select
| sql_injection.py:19:13:19:66 | BinaryExpr | sql_injection.py:9:15:9:21 | django.request.HttpRequest | sql_injection.py:19:13:19:66 | externally controlled string | This SQL query depends on $@. | sql_injection.py:9:15:9:21 | request | a user-provided value |
| sql_injection.py:22:38:22:91 | BinaryExpr | sql_injection.py:9:15:9:21 | django.request.HttpRequest | sql_injection.py:22:38:22:91 | externally controlled string | This SQL query depends on $@. | sql_injection.py:9:15:9:21 | request | a user-provided value |
| sql_injection.py:23:26:23:79 | BinaryExpr | sql_injection.py:9:15:9:21 | django.request.HttpRequest | sql_injection.py:23:26:23:79 | externally controlled string | This SQL query depends on $@. | sql_injection.py:9:15:9:21 | request | a user-provided value |
| sql_injection.py:24:28:24:81 | BinaryExpr | sql_injection.py:9:15:9:21 | django.request.HttpRequest | sql_injection.py:24:28:24:81 | externally controlled string | This SQL query depends on $@. | sql_injection.py:9:15:9:21 | request | a user-provided value |
| sql_injection.py:19:24:19:77 | BinaryExpr | sql_injection.py:12:24:12:31 | externally controlled string | sql_injection.py:19:24:19:77 | externally controlled string | This SQL query depends on $@. | sql_injection.py:12:24:12:31 | username | a user-provided value |
| sql_injection.py:22:38:22:95 | BinaryExpr | sql_injection.py:12:24:12:31 | externally controlled string | sql_injection.py:22:38:22:95 | externally controlled string | This SQL query depends on $@. | sql_injection.py:12:24:12:31 | username | a user-provided value |
| sql_injection.py:23:26:23:83 | BinaryExpr | sql_injection.py:12:24:12:31 | externally controlled string | sql_injection.py:23:26:23:83 | externally controlled string | This SQL query depends on $@. | sql_injection.py:12:24:12:31 | username | a user-provided value |
| sql_injection.py:24:28:24:85 | BinaryExpr | sql_injection.py:12:24:12:31 | externally controlled string | sql_injection.py:24:28:24:85 | externally controlled string | This SQL query depends on $@. | sql_injection.py:12:24:12:31 | username | a user-provided value |

48
python/ql/test/query-tests/Security/CWE-089/sql_injection.py
View File

@@ -1,28 +1,40 @@
"""This is copied from ql/python/ql/test/library-tests/web/django/test.py
and a only a slight extension of ql/python/ql/src/Security/CWE-089/examples/sql_injection.py
"""
from django.conf.urls import patterns, url
from django.conf.urls import url
from django.db import connection, models
from django.db.models.expressions import RawSQL
class Name(models.Model):
class User(models.Model):
pass
def save_name(request):
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,))
if request.method == 'POST':
name = request.POST.get('name')
curs = connection.cursor()
#GOOD -- Using parameters
curs.execute(
"insert into names_file ('name') values ('%s')", name)
#BAD -- Using string formatting
curs.execute(
"insert into names_file ('name') values ('%s')" % name)
# BAD -- Using string formatting
cursor.execute("SELECT * FROM users WHERE username = '%s'" % username)
#BAD -- other ways of executing raw SQL code with string interpolation
Name.objects.annotate(RawSQL("insert into names_file ('name') values ('%s')" % name))
Name.objects.raw("insert into names_file ('name') values ('%s')" % name)
Name.objects.extra("insert into names_file ('name') values ('%s')" % name)
# 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)
urlpatterns = patterns(url(r'^save_name/$',
save_name, name='save_name'))
# 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 = [url(r'^users/(?P<username>[^/]+)$', show_user)]