CVE-2020-7471: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') (CWE-89)
value: Django 1.11 before 1.11.28, 2.2 before 2.2.10, and 3.0 before 3.0.3 allows SQL Injection if untrusted data is used as a StringAgg delimiter (e.g., in Django applications that offer downloads of data as a series of rows with a user-specified column delimiter). By passing a suitably crafted delimiter to a contrib.postgres.aggregates.StringAgg instance, it was possible to break escaping and inject malicious SQL.
CWE description: The product constructs all or part of an SQL command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended SQL command when it is sent to a downstream component. Without sufficient removal or quoting of SQL syntax in user-controllable inputs, the generated SQL query can cause those inputs to be interpreted as SQL instead of ordinary user data.
CVSS v3 base score: 9.8
Published: 2020-02-03T12:15Z
Repository: https://github.com/django/django
Language: Other
Files changed: django/contrib/postgres/aggregates/general.py, django/contrib/postgres/aggregates/mixins.py, docs/releases/1.11.28.txt, docs/releases/2.2.10.txt, docs/releases/3.0.3.txt, docs/releases/index.txt, tests/postgres_tests/test_aggregates.py
Commit date: 2020-02-03 08:49:13 +0100
Commit message: Fixed CVE-2020-7471 -- Properly escaped StringAgg(delimiter) parameter.

Diff: diff --git a/django/contrib/postgres/aggregates/general.py b/django/contrib/postgres/aggregates/general.py
index 918373e926e..9616bc3e3e2 100644
--- a/django/contrib/postgres/aggregates/general.py
+++ b/django/contrib/postgres/aggregates/general.py
@@ -1,6 +1,7 @@
 from django.contrib.postgres.fields import ArrayField, JSONField
+from django.db.models import Value
 from django.db.models.aggregates import Aggregate
 
 from .mixins import OrderableAggMixin
 
 __all__ = [
@@ -49,15 +50,16 @@ class JSONBAgg(Aggregate):
         return value
 
 
 class StringAgg(OrderableAggMixin, Aggregate):
     function = 'STRING_AGG'
-    template = "%(function)s(%(distinct)s%(expressions)s, '%(delimiter)s'%(ordering)s)"
+    template = '%(function)s(%(distinct)s%(expressions)s %(ordering)s)'
     allow_distinct = True
 
     def __init__(self, expression, delimiter, **extra):
-        super().__init__(expression, delimiter=delimiter, **extra)
+        delimiter_expr = Value(str(delimiter))
+        super().__init__(expression, delimiter_expr, **extra)
 
     def convert_value(self, value, expression, connection):
         if not value:
             return ''
         return value
diff --git a/django/contrib/postgres/aggregates/mixins.py b/django/contrib/postgres/aggregates/mixins.py
index 3a43ca1a63f..82e8eebdf4c 100644
--- a/django/contrib/postgres/aggregates/mixins.py
+++ b/django/contrib/postgres/aggregates/mixins.py
@@ -1,20 +1,20 @@
 from django.db.models.expressions import F, OrderBy
 
 
 class OrderableAggMixin:
 
-    def __init__(self, expression, ordering=(), **extra):
+    def __init__(self, *expressions, ordering=(), **extra):
         if not isinstance(ordering, (list, tuple)):
             ordering = [ordering]
         ordering = ordering or []
         # Transform minus sign prefixed strings into an OrderBy() expression.
         ordering = (
             (OrderBy(F(o[1:]), descending=True) if isinstance(o, str) and o[0] == '-' else o)
             for o in ordering
         )
-        super().__init__(expression, **extra)
+        super().__init__(*expressions, **extra)
         self.ordering = self._parse_expressions(*ordering)
 
     def resolve_expression(self, *args, **kwargs):
         self.ordering = [expr.resolve_expression(*args, **kwargs) for expr in self.ordering]
         return super().resolve_expression(*args, **kwargs)
diff --git a/docs/releases/1.11.28.txt b/docs/releases/1.11.28.txt
new file mode 100644
index 00000000000..81ccb0ce06f
--- /dev/null
+++ b/docs/releases/1.11.28.txt
@@ -0,0 +1,13 @@
+============================
+Django 1.11.28 release notes
+============================
+
+*February 3, 2020*
+
+Django 1.11.28 fixes a security issue in 1.11.27.
+
+CVE-2020-7471: Potential SQL injection via ``StringAgg(delimiter)``
+===================================================================
+
+:class:`~django.contrib.postgres.aggregates.StringAgg` aggregation function was
+subject to SQL injection, using a suitably crafted ``delimiter``.
diff --git a/docs/releases/2.2.10.txt b/docs/releases/2.2.10.txt
new file mode 100644
index 00000000000..f82774dea09
--- /dev/null
+++ b/docs/releases/2.2.10.txt
@@ -0,0 +1,13 @@
+===========================
+Django 2.2.10 release notes
+===========================
+
+*February 3, 2020*
+
+Django 2.2.10 fixes a security issue in 2.2.9.
+
+CVE-2020-7471: Potential SQL injection via ``StringAgg(delimiter)``
+===================================================================
+
+:class:`~django.contrib.postgres.aggregates.StringAgg` aggregation function was
+subject to SQL injection, using a suitably crafted ``delimiter``.
diff --git a/docs/releases/3.0.3.txt b/docs/releases/3.0.3.txt
index ed92938e091..2eed2654c86 100644
--- a/docs/releases/3.0.3.txt
+++ b/docs/releases/3.0.3.txt
@@ -2,11 +2,17 @@
 Django 3.0.3 release notes
 ==========================
 
 *Expected February 3, 2020*
 
-Django 3.0.3 fixes several bugs in 3.0.2.
+Django 3.0.3 fixes a security issue and several bugs in 3.0.2.
+
+CVE-2020-7471: Potential SQL injection via ``StringAgg(delimiter)``
+===================================================================
+
+:class:`~django.contrib.postgres.aggregates.StringAgg` aggregation function was
+subject to SQL injection, using a suitably crafted ``delimiter``.
 
 Bugfixes
 ========
 
 * Fixed a regression in Django 3.0 that caused a crash when subtracting
diff --git a/docs/releases/index.txt b/docs/releases/index.txt
index 411ca4805ce..6a688587ef1 100644
--- a/docs/releases/index.txt
+++ b/docs/releases/index.txt
@@ -40,10 +40,11 @@ versions of the documentation contain the release notes for any later releases.
 2.2 release
 -----------
 .. toctree::
    :maxdepth: 1
 
+   2.2.10
    2.2.9
    2.2.8
    2.2.7
    2.2.6
    2.2.5
@@ -98,10 +99,11 @@ versions of the documentation contain the release notes for any later releases.
 1.11 release
 ------------
 .. toctree::
    :maxdepth: 1
 
+   1.11.28
    1.11.27
    1.11.26
    1.11.25
    1.11.24
    1.11.23
diff --git a/tests/postgres_tests/test_aggregates.py b/tests/postgres_tests/test_aggregates.py
index af84f12e916..a1dbe4441c7 100644
--- a/tests/postgres_tests/test_aggregates.py
+++ b/tests/postgres_tests/test_aggregates.py
@@ -167,10 +167,14 @@ class TestGeneralAggregate(PostgreSQLTestCase):
 
     def test_string_agg_requires_delimiter(self):
         with self.assertRaises(TypeError):
             AggregateTestModel.objects.aggregate(stringagg=StringAgg('char_field'))
 
+    def test_string_agg_delimiter_escaping(self):
+        values = AggregateTestModel.objects.aggregate(stringagg=StringAgg('char_field', delimiter="'"))
+        self.assertEqual(values, {'stringagg': "Foo1'Foo2'Foo4'Foo3"})
+
     def test_string_agg_charfield(self):
         values = AggregateTestModel.objects.aggregate(stringagg=StringAgg('char_field', delimiter=';'))
         self.assertEqual(values, {'stringagg': 'Foo1;Foo2;Foo4;Foo3'})
 
     def test_string_agg_charfield_ordering(self):

Vulnerable code:     template = "%(function)s(%(distinct)s%(expressions)s, '%(delimiter)s'%(ordering)s)"
        super().__init__(expression, delimiter=delimiter, **extra)
    def __init__(self, expression, ordering=(), **extra):
        super().__init__(expression, **extra)
Django 3.0.3 fixes several bugs in 3.0.2.
Fixed code: from django.db.models import Value
    template = '%(function)s(%(distinct)s%(expressions)s %(ordering)s)'
        delimiter_expr = Value(str(delimiter))
        super().__init__(expression, delimiter_expr, **extra)
    def __init__(self, *expressions, ordering=(), **extra):
        super().__init__(*expressions, **extra)
============================
Django 1.11.28 release notes
============================

*February 3, 2020*

Django 1.11.28 fixes a security issue in 1.11.27.

CVE-2020-7471: Potential SQL injection via ``StringAgg(delimiter)``
===================================================================

:class:`~django.contrib.postgres.aggregates.StringAgg` aggregation function was
subject to SQL injection, using a suitably crafted ``delimiter``.
===========================
Django 2.2.10 release notes
===========================

*February 3, 2020*

Django 2.2.10 fixes a security issue in 2.2.9.

CVE-2020-7471: Potential SQL injection via ``StringAgg(delimiter)``
===================================================================

:class:`~django.contrib.postgres.aggregates.StringAgg` aggregation function was
subject to SQL injection, using a suitably crafted ``delimiter``.
Django 3.0.3 fixes a security issue and several bugs in 3.0.2.

CVE-2020-7471: Potential SQL injection via ``StringAgg(delimiter)``
===================================================================

:class:`~django.contrib.postgres.aggregates.StringAgg` aggregation function was
subject to SQL injection, using a suitably crafted ``delimiter``.
   2.2.10
   1.11.28
    def test_string_agg_delimiter_escaping(self):
        values = AggregateTestModel.objects.aggregate(stringagg=StringAgg('char_field', delimiter="'"))
        self.assertEqual(values, {'stringagg': "Foo1'Foo2'Foo4'Foo3"})