Skip to content

Commit eb31d84

Browse files
charettescarltongibson
authored andcommitted
Fixed CVE-2020-7471 -- Properly escaped StringAgg(delimiter) parameter.
1 parent 6b178a3 commit eb31d84

File tree

7 files changed

+45
-5
lines changed

7 files changed

+45
-5
lines changed

Diff for: django/contrib/postgres/aggregates/general.py

+4-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from django.contrib.postgres.fields import ArrayField, JSONField
2+
from django.db.models import Value
23
from django.db.models.aggregates import Aggregate
34

45
from .mixins import OrderableAggMixin
@@ -51,11 +52,12 @@ def convert_value(self, value, expression, connection):
5152

5253
class StringAgg(OrderableAggMixin, Aggregate):
5354
function = 'STRING_AGG'
54-
template = "%(function)s(%(distinct)s%(expressions)s, '%(delimiter)s'%(ordering)s)"
55+
template = '%(function)s(%(distinct)s%(expressions)s %(ordering)s)'
5556
allow_distinct = True
5657

5758
def __init__(self, expression, delimiter, **extra):
58-
super().__init__(expression, delimiter=delimiter, **extra)
59+
delimiter_expr = Value(str(delimiter))
60+
super().__init__(expression, delimiter_expr, **extra)
5961

6062
def convert_value(self, value, expression, connection):
6163
if not value:

Diff for: django/contrib/postgres/aggregates/mixins.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
class OrderableAggMixin:
55

6-
def __init__(self, expression, ordering=(), **extra):
6+
def __init__(self, *expressions, ordering=(), **extra):
77
if not isinstance(ordering, (list, tuple)):
88
ordering = [ordering]
99
ordering = ordering or []
@@ -12,7 +12,7 @@ def __init__(self, expression, ordering=(), **extra):
1212
(OrderBy(F(o[1:]), descending=True) if isinstance(o, str) and o[0] == '-' else o)
1313
for o in ordering
1414
)
15-
super().__init__(expression, **extra)
15+
super().__init__(*expressions, **extra)
1616
self.ordering = self._parse_expressions(*ordering)
1717

1818
def resolve_expression(self, *args, **kwargs):

Diff for: docs/releases/1.11.28.txt

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
============================
2+
Django 1.11.28 release notes
3+
============================
4+
5+
*February 3, 2020*
6+
7+
Django 1.11.28 fixes a security issue in 1.11.27.
8+
9+
CVE-2020-7471: Potential SQL injection via ``StringAgg(delimiter)``
10+
===================================================================
11+
12+
:class:`~django.contrib.postgres.aggregates.StringAgg` aggregation function was
13+
subject to SQL injection, using a suitably crafted ``delimiter``.

Diff for: docs/releases/2.2.10.txt

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
===========================
2+
Django 2.2.10 release notes
3+
===========================
4+
5+
*February 3, 2020*
6+
7+
Django 2.2.10 fixes a security issue in 2.2.9.
8+
9+
CVE-2020-7471: Potential SQL injection via ``StringAgg(delimiter)``
10+
===================================================================
11+
12+
:class:`~django.contrib.postgres.aggregates.StringAgg` aggregation function was
13+
subject to SQL injection, using a suitably crafted ``delimiter``.

Diff for: docs/releases/3.0.3.txt

+7-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,13 @@ Django 3.0.3 release notes
44

55
*Expected February 3, 2020*
66

7-
Django 3.0.3 fixes several bugs in 3.0.2.
7+
Django 3.0.3 fixes a security issue and several bugs in 3.0.2.
8+
9+
CVE-2020-7471: Potential SQL injection via ``StringAgg(delimiter)``
10+
===================================================================
11+
12+
:class:`~django.contrib.postgres.aggregates.StringAgg` aggregation function was
13+
subject to SQL injection, using a suitably crafted ``delimiter``.
814

915
Bugfixes
1016
========

Diff for: docs/releases/index.txt

+2
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ versions of the documentation contain the release notes for any later releases.
4242
.. toctree::
4343
:maxdepth: 1
4444

45+
2.2.10
4546
2.2.9
4647
2.2.8
4748
2.2.7
@@ -100,6 +101,7 @@ versions of the documentation contain the release notes for any later releases.
100101
.. toctree::
101102
:maxdepth: 1
102103

104+
1.11.28
103105
1.11.27
104106
1.11.26
105107
1.11.25

Diff for: tests/postgres_tests/test_aggregates.py

+4
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,10 @@ def test_string_agg_requires_delimiter(self):
169169
with self.assertRaises(TypeError):
170170
AggregateTestModel.objects.aggregate(stringagg=StringAgg('char_field'))
171171

172+
def test_string_agg_delimiter_escaping(self):
173+
values = AggregateTestModel.objects.aggregate(stringagg=StringAgg('char_field', delimiter="'"))
174+
self.assertEqual(values, {'stringagg': "Foo1'Foo2'Foo4'Foo3"})
175+
172176
def test_string_agg_charfield(self):
173177
values = AggregateTestModel.objects.aggregate(stringagg=StringAgg('char_field', delimiter=';'))
174178
self.assertEqual(values, {'stringagg': 'Foo1;Foo2;Foo4;Foo3'})

0 commit comments

Comments
 (0)