Browse Source

Fixed #28770 -- Warned that quoting a placeholder in a raw SQL string is unsafe.

Thanks Hynek Cernoch for the report and review.
Tim Graham 7 years ago
parent
commit
327f0f37ce
3 changed files with 38 additions and 14 deletions
  1. 13 5
      docs/ref/models/expressions.txt
  2. 9 2
      docs/ref/models/querysets.txt
  3. 16 7
      docs/topics/db/sql.txt

+ 13 - 5
docs/ref/models/expressions.txt

@@ -654,11 +654,19 @@ should avoid them if possible.
 
 
 .. warning::
 .. warning::
 
 
-    You should be very careful to escape any parameters that the user can
-    control by using ``params`` in order to protect against :ref:`SQL injection
-    attacks <sql-injection-protection>`. ``params`` is a required argument to
-    force you to acknowledge that you're not interpolating your SQL with user
-    provided data.
+    To protect against `SQL injection attacks
+    <https://en.wikipedia.org/wiki/SQL_injection>`_, you must escape any
+    parameters that the user can control by using ``params``. ``params`` is a
+    required argument to force you to acknowledge that you're not interpolating
+    your SQL with user-provided data.
+
+    You also must not quote placeholders in the SQL string. This example is
+    vulnerable to SQL injection because of the quotes around ``%s``::
+
+        RawSQL("select col from sometable where othercol = '%s'")  # unsafe!
+
+    You can read more about how Django's :ref:`SQL injection protection
+    <sql-injection-protection>` works.
 
 
 Window functions
 Window functions
 ----------------
 ----------------

+ 9 - 2
docs/ref/models/querysets.txt

@@ -1262,8 +1262,15 @@ generated by a ``QuerySet``.
 
 
     You should be very careful whenever you use ``extra()``. Every time you use
     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
     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 :ref:`SQL injection protection <sql-injection-protection>`.
+    ``params`` in order to protect against SQL injection attacks.
+
+    You also must not quote placeholders in the SQL string. This example is
+    vulnerable to SQL injection because of the quotes around ``%s``::
+
+        "select col from sometable where othercol = '%s'"  # unsafe!
+
+    You can read more about how Django's :ref:`SQL injection protection
+    <sql-injection-protection>` works.
 
 
 By definition, these extra lookups may not be portable to different database
 By definition, these extra lookups may not be portable to different database
 engines (because you're explicitly writing SQL code) and violate the DRY
 engines (because you're explicitly writing SQL code) and violate the DRY

+ 16 - 7
docs/topics/db/sql.txt

@@ -210,20 +210,26 @@ argument.
 
 
 .. warning::
 .. warning::
 
 
-    **Do not use string formatting on raw queries!**
+    **Do not use string formatting on raw queries or quote placeholders in your
+    SQL strings!**
 
 
     It's tempting to write the above query as::
     It's tempting to write the above query as::
 
 
         >>> query = 'SELECT * FROM myapp_person WHERE last_name = %s' % lname
         >>> query = 'SELECT * FROM myapp_person WHERE last_name = %s' % lname
         >>> Person.objects.raw(query)
         >>> Person.objects.raw(query)
 
 
-    **Don't.**
+    You might also think you should write your query like this (with quotes
+    around ``%s``)::
 
 
-    Using the ``params`` argument completely protects you from `SQL injection
-    attacks`__, a common exploit where attackers inject arbitrary SQL into
-    your database. If you use string interpolation, sooner or later you'll
-    fall victim to SQL injection. As long as you remember to always use the
-    ``params`` argument you'll be protected.
+        >>> query = "SELECT * FROM myapp_person WHERE last_name = '%s'"
+
+    **Don't make either of these mistakes.**
+
+    As discussed in :ref:`sql-injection-protection`, using the ``params``
+    argument and leaving the placeholders unquoted protects you from `SQL
+    injection attacks`__, a common exploit where attackers inject arbitrary
+    SQL into your database. If you use string interpolation or quote the
+    placeholder, you're at risk for SQL injection.
 
 
 __ https://en.wikipedia.org/wiki/SQL_injection
 __ https://en.wikipedia.org/wiki/SQL_injection
 
 
@@ -257,6 +263,9 @@ For example::
 
 
         return row
         return row
 
 
+To protect against SQL injection, you must not include quotes around the ``%s``
+placeholders in the SQL string.
+
 Note that if you want to include literal percent signs in the query, you have to
 Note that if you want to include literal percent signs in the query, you have to
 double them in the case you are passing parameters::
 double them in the case you are passing parameters::