From 001b0634cd309e372edb6d7d95d083d02b8e37bd Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Wed, 22 Jan 2020 09:03:27 +0100 Subject: [PATCH] [1.11.x] Fixed CVE-2020-7471 -- Properly escaped StringAgg(delimiter) parameter. --- django/contrib/postgres/aggregates/general.py | 6 ++++-- docs/releases/1.11.28.txt | 13 +++++++++++++ docs/releases/index.txt | 1 + tests/postgres_tests/test_aggregates.py | 4 ++++ 4 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 docs/releases/1.11.28.txt diff --git a/django/contrib/postgres/aggregates/general.py b/django/contrib/postgres/aggregates/general.py index 5b3d22bf98..cd9a4bc349 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 JSONField +from django.db.models import Value from django.db.models.aggregates import Aggregate __all__ = [ @@ -43,11 +44,12 @@ class JSONBAgg(Aggregate): class StringAgg(Aggregate): function = 'STRING_AGG' - template = "%(function)s(%(distinct)s%(expressions)s, '%(delimiter)s')" + template = '%(function)s(%(distinct)s%(expressions)s)' def __init__(self, expression, delimiter, distinct=False, **extra): distinct = 'DISTINCT ' if distinct else '' - super(StringAgg, self).__init__(expression, delimiter=delimiter, distinct=distinct, **extra) + delimiter_expr = Value(str(delimiter)) + super(StringAgg, self).__init__(expression, delimiter_expr, distinct=distinct, **extra) def convert_value(self, value, expression, connection, context): if not value: 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/index.txt b/docs/releases/index.txt index cd13f8695e..c2e913cafc 100644 --- a/docs/releases/index.txt +++ b/docs/releases/index.txt @@ -26,6 +26,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 9aa0e06595..ddfd7fce26 100644 --- a/tests/postgres_tests/test_aggregates.py +++ b/tests/postgres_tests/test_aggregates.py @@ -108,6 +108,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'Foo3'Foo4"}) + def test_string_agg_charfield(self): values = AggregateTestModel.objects.aggregate(stringagg=StringAgg('char_field', delimiter=';')) self.assertEqual(values, {'stringagg': 'Foo1;Foo2;Foo3;Foo4'})