Re: [Django] #28770: Warn that quoting a placeholder in a raw SQL string can lead to SQL injection

[Django]

#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:  closed
Component:  Documentation |  Version:  master
 Severity:  Normal|   Resolution:  fixed
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by Tim Graham ):

 In [changeset:"c869207ea29822c81d5a242a1e401135d813a96c" c869207e]:
 {{{
 #!CommitTicketReference repository=""
 revision="c869207ea29822c81d5a242a1e401135d813a96c"
 [2.0.x] Fixed #28770 -- Warned that quoting a placeholder in a raw SQL
 string is unsafe.

 Thanks Hynek Cernoch for the report and review.

 Backport of 327f0f37ce3c1e5ac3a19668add237ddd92266d6 from master
 }}}

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.5d780dd272b3e23091d5d2528c458442%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28770: Warn that quoting a placeholder in a raw SQL string can lead to SQL injection

[Django]

#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:  closed
Component:  Documentation |  Version:  master
 Severity:  Normal|   Resolution:  fixed
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+
Changes (by GitHub ):

 * status:  new => closed
 * resolution:   => fixed


Comment:

 In [changeset:"327f0f37ce3c1e5ac3a19668add237ddd92266d6" 327f0f3]:
 {{{
 #!CommitTicketReference repository=""
 revision="327f0f37ce3c1e5ac3a19668add237ddd92266d6"
 Fixed #28770 -- Warned that quoting a placeholder in a raw SQL string is
 unsafe.

 Thanks Hynek Cernoch for the report and review.
 }}}

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.385159e64d67f9a4801b40aa179f3f0d%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28770: Warn that quoting a placeholder in a raw SQL string can lead to SQL injection

[Django]

#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|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+
Changes (by Tim Graham):

 * has_patch:  0 => 1


Comment:

 [https://github.com/django/django/pull/9327 PR]

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.c42782608fac13fac571ac130cd44e1e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

[Django] #28770: Warn that quoting a placeholder in a raw SQL string can lead to SQL injection

[Django]

#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: 
Django 
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