Patch for 1.11.27

django/contrib/gis/db/models/aggregates.py

@@ -1,4 +1,5 @@
 from django.contrib.gis.db.models.fields import ExtentField
+from django.db.models import Value
 from django.db.models.aggregates import Aggregate
 
 __all__ = ['Collect', 'Extent', 'Extent3D', 'MakeLine', 'Union']
@@ -16,11 +17,14 @@ def as_sql(self, compiler, connection):
         return super(GeoAggregate, self).as_sql(compiler, connection)
 
     def as_oracle(self, compiler, connection):
-        if not hasattr(self, 'tolerance'):
-            self.tolerance = 0.05
-        self.extra['tolerance'] = self.tolerance
         if not self.is_extent:
-            self.template = '%(function)s(SDOAGGRTYPE(%(expressions)s,%(tolerance)s))'
+            tolerance = self.extra.get('tolerance') or getattr(self, 'tolerance', 0.05)
+            clone = self.copy()
+            expressions = clone.get_source_expressions()
+            expressions.append(Value(tolerance))
+            clone.set_source_expressions(expressions)
+            clone.template = '%(function)s(SDOAGGRTYPE(%(expressions)s))'
+            return clone.as_sql(compiler, connection)
         return self.as_sql(compiler, connection)
 
     def resolve_expression(self, query=None, allow_joins=True, reuse=None, summarize=False, for_save=False):

django/contrib/gis/db/models/functions.py

@@ -117,7 +117,11 @@ class OracleToleranceMixin(object):
     tolerance = 0.05
 
     def as_oracle(self, compiler, connection):
-        tol = self.extra.get('tolerance', self.tolerance)
+        tol = self._handle_param(
+            self.extra.get('tolerance', self.tolerance),
+            'tolerance',
+            NUMERIC_TYPES,
+        )
         self.template = "%%(function)s(%%(expressions)s, %s)" % tol
         return super(OracleToleranceMixin, self).as_sql(compiler, connection)
 

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 @@ def convert_value(self, value, expression, connection, context):
 
 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:

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``.

docs/releases/1.11.29.txt

@@ -0,0 +1,13 @@
+============================
+Django 1.11.29 release notes
+============================
+
+*March 4, 2020*
+
+Django 1.11.29 fixes a security issue in 1.11.29.
+
+CVE-2020-9402: Potential SQL injection via ``tolerance`` parameter in GIS functions and aggregates on Oracle
+============================================================================================================
+
+GIS functions and aggregates on Oracle were subject to SQL injection,
+using a suitably crafted ``tolerance``.

docs/releases/index.txt

@@ -26,6 +26,8 @@ versions of the documentation contain the release notes for any later releases.
 .. toctree::
    :maxdepth: 1
 
+   1.11.29
+   1.11.28
    1.11.27
    1.11.26
    1.11.25

tests/gis_tests/distapp/tests.py

@@ -1,6 +1,6 @@
 from __future__ import unicode_literals
 
-from unittest import skipIf
+from unittest import skipIf, skipUnless
 
 from django.contrib.gis.db.models.functions import (
     Area, Distance, Length, Perimeter, Transform,
@@ -588,6 +588,37 @@ def test_distance_geodetic_spheroid(self):
             for i, c in enumerate(qs):
                 self.assertAlmostEqual(sphere_distances[i], c.distance.m, tol)
 
+    @skipUnless(
+        connection.vendor == 'oracle',
+        'Oracle supports tolerance paremeter.',
+    )
+    def test_distance_function_tolerance_escaping(self):
+        qs = AustraliaCity.objects.annotate(
+            d=Distance(
+                'point',
+                Point(0, 0, srid=3857),
+                tolerance='0.05) = 1 OR 1=1 OR (1+1',
+            ),
+        ).filter(d=1).values('pk')
+        msg = 'The tolerance parameter has the wrong type'
+        with self.assertRaisesMessage(TypeError, msg):
+            qs.exists()
+
+    @skipUnless(
+        connection.vendor == 'oracle',
+        'Oracle supports tolerance paremeter.',
+    )
+    def test_distance_function_tolerance(self):
+        # Tolerance is greater than distance.
+        qs = AustraliaCity.objects.annotate(
+            d=Distance(
+                'point',
+                Point(151.23, -33.95, srid=4326),
+                tolerance=340.7,
+            ),
+        ).filter(d=0).values('pk')
+        self.assertIs(qs.exists(), True)
+
     @no_oracle  # Oracle already handles geographic distance calculation.
     @skipUnlessDBFeature("has_Distance_function", 'has_Transform_function')
     def test_distance_transform(self):

tests/gis_tests/geoapp/tests.py

@@ -2,6 +2,7 @@
 
 import re
 import tempfile
+import unittest
 
 from django.contrib.gis import gdal
 from django.contrib.gis.db.models import Extent, MakeLine, Union
@@ -10,7 +11,7 @@
     MultiPoint, MultiPolygon, Point, Polygon, fromstr,
 )
 from django.core.management import call_command
-from django.db import connection
+from django.db import DatabaseError, connection
 from django.test import TestCase, ignore_warnings, skipUnlessDBFeature
 from django.utils import six
 from django.utils.deprecation import RemovedInDjango20Warning
@@ -881,6 +882,42 @@ def test_unionagg(self):
         qs = City.objects.filter(name='NotACity')
         self.assertIsNone(qs.aggregate(Union('point'))['point__union'])
 
+    @unittest.skipUnless(
+        connection.vendor == 'oracle',
+        'Oracle supports tolerance paremeter.',
+    )
+    def test_unionagg_tolerance(self):
+        City.objects.create(
+            point=fromstr('POINT(-96.467222 32.751389)', srid=4326),
+            name='Forney',
+        )
+        tx = Country.objects.get(name='Texas').mpoly
+        # Tolerance is greater than distance between Forney and Dallas, that's
+        # why Dallas is ignored.
+        forney_houston = GEOSGeometry(
+            'MULTIPOINT(-95.363151 29.763374, -96.467222 32.751389)',
+            srid=4326,
+        )
+        self.assertIs(
+            forney_houston.equals(
+                City.objects.filter(point__within=tx).aggregate(
+                    Union('point', tolerance=32000),
+                )['point__union'],
+            ),
+            True,
+        )
+
+    @unittest.skipUnless(
+        connection.vendor == 'oracle',
+        'Oracle supports tolerance paremeter.',
+    )
+    def test_unionagg_tolerance_escaping(self):
+        tx = Country.objects.get(name='Texas').mpoly
+        with self.assertRaises(DatabaseError):
+            City.objects.filter(point__within=tx).aggregate(
+                Union('point', tolerance='0.05))), (((1'),
+            )
+
     def test_within_subquery(self):
         """
         Using a queryset inside a geo lookup is working (using a subquery)

tests/postgres_tests/test_aggregates.py

@@ -108,6 +108,10 @@ 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'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'})