From eb31d845323618d688ad429479c6dda973056136 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Tue, 31 Dec 2019 12:46:06 -0500 Subject: [PATCH] Fixed CVE-2020-7471 -- Properly escaped StringAgg(delimiter) parameter. --- django/contrib/postgres/aggregates/general.py | 6 ++++-- django/contrib/postgres/aggregates/mixins.py | 4 ++-- docs/releases/1.11.28.txt | 13 +++++++++++++ docs/releases/2.2.10.txt | 13 +++++++++++++ docs/releases/3.0.3.txt | 8 +++++++- docs/releases/index.txt | 2 ++ tests/postgres_tests/test_aggregates.py | 4 ++++ 7 files changed, 45 insertions(+), 5 deletions(-) create mode 100644 docs/releases/1.11.28.txt create mode 100644 docs/releases/2.2.10.txt diff --git a/django/contrib/postgres/aggregates/general.py b/django/contrib/postgres/aggregates/general.py index 918373e926..9616bc3e3e 100644 --- a/django/contrib/postgres/aggregates/general.py +++ b/django/contrib/postgres/aggregates/general.py @@ -1,4 +1,5 @@ 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 @@ -51,11 +52,12 @@ class JSONBAgg(Aggregate): 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: diff --git a/django/contrib/postgres/aggregates/mixins.py b/django/contrib/postgres/aggregates/mixins.py index 3a43ca1a63..82e8eebdf4 100644 --- a/django/contrib/postgres/aggregates/mixins.py +++ b/django/contrib/postgres/aggregates/mixins.py @@ -3,7 +3,7 @@ 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 [] @@ -12,7 +12,7 @@ class OrderableAggMixin: (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): diff --git a/docs/releases/1.11.28.txt b/docs/releases/1.11.28.txt new file mode 100644 index 0000000000..81ccb0ce06 --- /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 0000000000..f82774dea0 --- /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 ed92938e09..2eed2654c8 100644 --- a/docs/releases/3.0.3.txt +++ b/docs/releases/3.0.3.txt @@ -4,7 +4,13 @@ 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 ======== diff --git a/docs/releases/index.txt b/docs/releases/index.txt index 411ca4805c..6a688587ef 100644 --- a/docs/releases/index.txt +++ b/docs/releases/index.txt @@ -42,6 +42,7 @@ versions of the documentation contain the release notes for any later releases. .. toctree:: :maxdepth: 1 + 2.2.10 2.2.9 2.2.8 2.2.7 @@ -100,6 +101,7 @@ versions of the documentation contain the release notes for any later releases. .. toctree:: :maxdepth: 1 + 1.11.28 1.11.27 1.11.26 1.11.25 diff --git a/tests/postgres_tests/test_aggregates.py b/tests/postgres_tests/test_aggregates.py index af84f12e91..a1dbe4441c 100644 --- a/tests/postgres_tests/test_aggregates.py +++ b/tests/postgres_tests/test_aggregates.py @@ -169,6 +169,10 @@ class TestGeneralAggregate(PostgreSQLTestCase): 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'})