From 7d3c02a214ae7f59d56c8620b3bb31728d04cd99 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Wed, 4 Oct 2017 11:10:38 -0400 Subject: [PATCH] [2.0.x] Fixed #28680 -- Doc'd Func.__init__()'s **extra and as_sql()'s **extra_context aren't escaped. Thanks Hynek Cernoch for the report and review. Backport of 1e7dbbdec5ca2bd236b8ab64fa172a8fc0abcb1e from master --- docs/ref/models/expressions.txt | 57 ++++++++++++++++++++++++++++++--- 1 file changed, 53 insertions(+), 4 deletions(-) diff --git a/docs/ref/models/expressions.txt b/docs/ref/models/expressions.txt index 6ef1001a9ff..f9dec50a5db 100644 --- a/docs/ref/models/expressions.txt +++ b/docs/ref/models/expressions.txt @@ -306,6 +306,11 @@ The ``Func`` API is as follows: template="%(function)s('', %(expressions)s)", ) + To avoid a SQL injection vulnerability, ``extra_context`` :ref:`must + not contain untrusted user input ` + as these values are interpolated into the SQL string rather than passed + as query parameters, where the database driver would escape them. + The ``*expressions`` argument is a list of positional expressions that the function will be applied to. The expressions will be converted to strings, joined together with ``arg_joiner``, and then interpolated into the ``template`` @@ -316,10 +321,15 @@ assumed to be column references and will be wrapped in ``F()`` expressions while other values will be wrapped in ``Value()`` expressions. The ``**extra`` kwargs are ``key=value`` pairs that can be interpolated -into the ``template`` attribute. The ``function``, ``template``, and -``arg_joiner`` keywords can be used to replace the attributes of the same name -without having to define your own class. ``output_field`` can be used to define -the expected return type. +into the ``template`` attribute. To avoid a SQL injection vulnerability, +``extra`` :ref:`must not contain untrusted user input +` as these values are interpolated +into the SQL string rather than passed as query parameters, where the database +driver would escape them. + +The ``function``, ``template``, and ``arg_joiner`` keywords can be used to +replace the attributes of the same name without having to define your own +class. ``output_field`` can be used to define the expected return type. ``Aggregate()`` expressions --------------------------- @@ -1081,6 +1091,45 @@ Let's see how it works:: Yahoo: Internet Company Django Software Foundation: No Tagline +.. _avoiding-sql-injection-in-query-expressions: + +Avoiding SQL injection +~~~~~~~~~~~~~~~~~~~~~~ + +Since a ``Func``'s keyword arguments for ``__init__()`` (``**extra``) and +``as_sql()`` (``**extra_context``) are interpolated into the SQL string rather +than passed as query parameters (where the database driver would escape them), +they must not contain untrusted user input. + +For example, if ``substring`` is user-provided, this function is vulnerable to +SQL injection:: + + from django.db.models import Func + + class Position(Func): + function = 'POSITION' + template = "%(function)s('%(substring)s' in %(expressions)s)" + + def __init__(self, expression, substring): + # substring=substring is a SQL injection vulnerability! + super().__init__(expression, substring=substring) + +This function generates a SQL string without any parameters. Since ``substring`` +is passed to ``super().__init__()`` as a keyword argument, it's interpolated +into the SQL string before the query is sent to the database. + +Here's a corrected rewrite:: + + class Position(Func): + function = 'POSITION' + arg_joiner = ' IN ' + + def __init__(self, expression, substring): + super().__init__(substring, expression) + +With ``substring`` instead passed as a positional argument, it'll be passed as +a parameter in the database query. + Adding support in third-party database backends -----------------------------------------------