#28770: Warn that quoting a placeholder in a raw SQL string can lead to SQL
injection
------------------------------------------------+------------------------
Reporter: Hynek Cernoch | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Severity: Normal | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
This is a follow up to #28680 which addresses the the security of `Func()`
query expression if used with unsafe `extra` or `extra_context`.
I have found a similar issue in:
{{{
models.query.QuerySet.extra(... params=...)
models.expressions.RawSQL(... params=...)
models.Manager.raw(... params=...)
cursor.execute(... params=...)
}}}
if the SQL is constructed carelessly with apostrophes around the parameter
placeholder " '%s' " and the parameter was obtained from a user input by:
a) by JSON without a type control, or
b) by guessing the type
c) with an obsoleted unused parameter in a more complicated query that has
not been ever tested.
Example:
{{{
from django.contrib.auth.models import User
class Data(models.Model):
user = models.ForeignKey(User, models.DO_NOTHING)
option = models.CharField(max_length=20)
val = models.IntegerField()
secret = models.CharField(max_length=100)
}}}
--- view common part ---
{{{
# common part - get Data for the current authenticated user
qs = Data.objects.filter(user=request.user)
# attack to get all Data
}}}
A) vulnerable code - added filter by JSON data without a type control
{{{
user_data = """{"val": 1}""" # expected example
user_data = """{"val": ") OR TRUE OR (val="}"""' # attack example
val= json.loads(json_data)['val']
qs = qs.extra(where=["val='%s'"], params=[val]])
}}}
B) vulnerable code - guess a type (very similar)
{{{
user_data = "1" # expected example
user_data = ") OR TRUE OR (val="# attack example
val = int(user_data) if user_data.isdigit() else user_data
qs = qs.extra(where=["val='%s'"], params=[val])
}}}
C) vulnerable due to an unused obsoleted request variable 'user_data'
{{{
user_data = '' # usual value of an unused
field
user_data = ')); DROP TABLE ...; --' # attack example
qs = qs.extra(where=["val > 0 OR val < 0 AND option='%s'"],
params=[user_data])
# if negative values `val` are not expected seriously then the query
could be untested enough
}}}
Backends:
- mysql - all A, B, C) vulnerable
- postgresql - vulnerable C)
- sqlite3 - seems safe
A complete code with a test is in attachment.
If I compare these vulnerabilities with #28680 Func(), it is less probable
that the invalid SQL will work and some circumstances like A), B) or C)
are necessary. On the other hand these functions are probably much more
used (by my experience at StackOverflow)
Generally, escaping of parameters by a database driver is a safe technique
if the SQL is carefully constructed. That is still OK for ORM generated
queries, but generally not for manually constructed parts of SQL like in
the functions above.
A separation of SQL and data by parameterized queries is also a safe
technique if it is done consistently from the beginning up to a
parameterized call to database server. But it is not a case of most Django
db drivers.
I expect that also this issue will be solved by documentation first. A
security scanner for SQL can be used later. It can identify suspicious SQL
at run-time in debug mode and write warnings.
The
[https://docs.djangoproject.com/en/dev/ref/models/querysets/#django.db.models.query.QuerySet.extra
current docs for extra()]:
> Warning:
> You should be very careful whenever you use extra(). Every time you use
it, you should escape any parameters that the user can control by using
params in order to protect against SQL injection attacks . Please read
more about SQL injection protection.
Similar texts are for `RawSQL()` and `raw()`.
something similar to [http://initd.org/psycopg/docs/usage.html#the-
problem-with-the-query-parameters psycopg's docs] is useful:
... ('%s');" # NEVER DO THIS
... (%s);" # Note: no quotes
This can be reported as an issue to driver development. A check there is
more effective.
--
Ticket URL: <https://code.djangoproject.com/ticket/28770>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/051.785c52c6dc27a41cdcc6e102094efe1a%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
