diff --git a/django/contrib/gis/db/models/aggregates.py b/django/contrib/gis/db/models/aggregates.py index 69380beea9..3aa46a52c0 100644 --- a/django/contrib/gis/db/models/aggregates.py +++ b/django/contrib/gis/db/models/aggregates.py @@ -1,7 +1,7 @@ from django.contrib.gis.db.models.fields import ( ExtentField, GeometryCollectionField, GeometryField, LineStringField, ) -from django.db.models import Aggregate +from django.db.models import Aggregate, Value from django.utils.functional import cached_property __all__ = ['Collect', 'Extent', 'Extent3D', 'MakeLine', 'Union'] @@ -27,9 +27,16 @@ class GeoAggregate(Aggregate): ) def as_oracle(self, compiler, connection, **extra_context): - tolerance = self.extra.get('tolerance') or getattr(self, 'tolerance', 0.05) - template = None if self.is_extent else '%(function)s(SDOAGGRTYPE(%(expressions)s,%(tolerance)s))' - return self.as_sql(compiler, connection, template=template, tolerance=tolerance, **extra_context) + if not self.is_extent: + tolerance = self.extra.get('tolerance') or getattr(self, 'tolerance', 0.05) + clone = self.copy() + clone.set_source_expressions([ + *self.get_source_expressions(), + Value(tolerance), + ]) + template = '%(function)s(SDOAGGRTYPE(%(expressions)s))' + return clone.as_sql(compiler, connection, template=template, **extra_context) + return self.as_sql(compiler, connection, **extra_context) def resolve_expression(self, query=None, allow_joins=True, reuse=None, summarize=False, for_save=False): c = super().resolve_expression(query, allow_joins, reuse, summarize, for_save) diff --git a/django/contrib/gis/db/models/functions.py b/django/contrib/gis/db/models/functions.py index 9734068574..de4ee3d6bd 100644 --- a/django/contrib/gis/db/models/functions.py +++ b/django/contrib/gis/db/models/functions.py @@ -111,12 +111,14 @@ class OracleToleranceMixin: tolerance = 0.05 def as_oracle(self, compiler, connection, **extra_context): - tol = self.extra.get('tolerance', self.tolerance) - return self.as_sql( - compiler, connection, - template="%%(function)s(%%(expressions)s, %s)" % tol, - **extra_context - ) + tolerance = Value(self._handle_param( + self.extra.get('tolerance', self.tolerance), + 'tolerance', + NUMERIC_TYPES, + )) + clone = self.copy() + clone.set_source_expressions([*self.get_source_expressions(), tolerance]) + return clone.as_sql(compiler, connection, **extra_context) class Area(OracleToleranceMixin, GeoFunc): diff --git a/docs/releases/1.11.29.txt b/docs/releases/1.11.29.txt new file mode 100644 index 0000000000..d37f3ffc0d --- /dev/null +++ b/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``. diff --git a/docs/releases/2.2.11.txt b/docs/releases/2.2.11.txt index b14d961ac3..9738ef4470 100644 --- a/docs/releases/2.2.11.txt +++ b/docs/releases/2.2.11.txt @@ -2,9 +2,15 @@ Django 2.2.11 release notes =========================== -*Expected March 2, 2020* +*March 4, 2020* -Django 2.2.11 fixes a data loss bug in 2.2.10. +Django 2.2.11 fixes a security issue and a data loss bug in 2.2.10. + +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``. Bugfixes ======== diff --git a/docs/releases/3.0.4.txt b/docs/releases/3.0.4.txt index ad9752addb..647e593749 100644 --- a/docs/releases/3.0.4.txt +++ b/docs/releases/3.0.4.txt @@ -2,9 +2,15 @@ Django 3.0.4 release notes ========================== -*Expected March 2, 2020* +*March 4, 2020* -Django 3.0.4 fixes several bugs in 3.0.3. +Django 3.0.4 fixes a security issue and several bugs in 3.0.3. + +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``. Bugfixes ======== diff --git a/docs/releases/index.txt b/docs/releases/index.txt index 3c56efd533..f03856c2f2 100644 --- a/docs/releases/index.txt +++ b/docs/releases/index.txt @@ -103,6 +103,7 @@ 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 diff --git a/tests/gis_tests/distapp/tests.py b/tests/gis_tests/distapp/tests.py index 2cdd0e8f0e..4e2c95ee18 100644 --- a/tests/gis_tests/distapp/tests.py +++ b/tests/gis_tests/distapp/tests.py @@ -434,6 +434,37 @@ class DistanceFunctionsTests(FuncTestMixin, TestCase): ).filter(d=D(m=1)) self.assertTrue(qs.exists()) + @unittest.skipUnless( + connection.vendor == 'oracle', + 'Oracle supports tolerance paremeter.', + ) + def test_distance_function_tolerance_escaping(self): + qs = Interstate.objects.annotate( + d=Distance( + Point(500, 500, srid=3857), + Point(0, 0, srid=3857), + tolerance='0.05) = 1 OR 1=1 OR (1+1', + ), + ).filter(d=D(m=1)).values('pk') + msg = 'The tolerance parameter has the wrong type' + with self.assertRaisesMessage(TypeError, msg): + qs.exists() + + @unittest.skipUnless( + connection.vendor == 'oracle', + 'Oracle supports tolerance paremeter.', + ) + def test_distance_function_tolerance(self): + # Tolerance is greater than distance. + qs = Interstate.objects.annotate( + d=Distance( + Point(0, 0, srid=3857), + Point(1, 1, srid=3857), + tolerance=1.5, + ), + ).filter(d=0).values('pk') + self.assertIs(qs.exists(), True) + @skipIfDBFeature("supports_distance_geodetic") @skipUnlessDBFeature("has_Distance_function") def test_distance_function_raw_result_d_lookup(self): diff --git a/tests/gis_tests/geoapp/tests.py b/tests/gis_tests/geoapp/tests.py index f66e315e73..b72b9b8949 100644 --- a/tests/gis_tests/geoapp/tests.py +++ b/tests/gis_tests/geoapp/tests.py @@ -9,7 +9,7 @@ from django.contrib.gis.geos import ( MultiPoint, MultiPolygon, Point, Polygon, fromstr, ) from django.core.management import call_command -from django.db import NotSupportedError, connection +from django.db import DatabaseError, NotSupportedError, connection from django.db.models import F, OuterRef, Subquery from django.test import TestCase, skipUnlessDBFeature @@ -594,6 +594,42 @@ class GeoQuerySetTest(TestCase): 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)