Fixed CVE-2020-9402 -- Properly escaped tolerance parameter in GIS functions and aggregates on Oracle.

Thanks to Norbert Szetei for the report.
This commit is contained in:
Mariusz Felisiak 2020-02-24 14:46:28 +01:00
parent 65ab4f9f03
commit 6695d29b1c
8 changed files with 117 additions and 15 deletions

View File

@ -1,7 +1,7 @@
from django.contrib.gis.db.models.fields import ( from django.contrib.gis.db.models.fields import (
ExtentField, GeometryCollectionField, GeometryField, LineStringField, ExtentField, GeometryCollectionField, GeometryField, LineStringField,
) )
from django.db.models import Aggregate from django.db.models import Aggregate, Value
from django.utils.functional import cached_property from django.utils.functional import cached_property
__all__ = ['Collect', 'Extent', 'Extent3D', 'MakeLine', 'Union'] __all__ = ['Collect', 'Extent', 'Extent3D', 'MakeLine', 'Union']
@ -27,9 +27,16 @@ class GeoAggregate(Aggregate):
) )
def as_oracle(self, compiler, connection, **extra_context): def as_oracle(self, compiler, connection, **extra_context):
if not self.is_extent:
tolerance = self.extra.get('tolerance') or getattr(self, 'tolerance', 0.05) 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))' clone = self.copy()
return self.as_sql(compiler, connection, template=template, tolerance=tolerance, **extra_context) 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): 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) c = super().resolve_expression(query, allow_joins, reuse, summarize, for_save)

View File

@ -111,12 +111,14 @@ class OracleToleranceMixin:
tolerance = 0.05 tolerance = 0.05
def as_oracle(self, compiler, connection, **extra_context): def as_oracle(self, compiler, connection, **extra_context):
tol = self.extra.get('tolerance', self.tolerance) tolerance = Value(self._handle_param(
return self.as_sql( self.extra.get('tolerance', self.tolerance),
compiler, connection, 'tolerance',
template="%%(function)s(%%(expressions)s, %s)" % tol, NUMERIC_TYPES,
**extra_context ))
) clone = self.copy()
clone.set_source_expressions([*self.get_source_expressions(), tolerance])
return clone.as_sql(compiler, connection, **extra_context)
class Area(OracleToleranceMixin, GeoFunc): class Area(OracleToleranceMixin, GeoFunc):

13
docs/releases/1.11.29.txt Normal file
View File

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

View File

@ -2,9 +2,15 @@
Django 2.2.11 release notes 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 Bugfixes
======== ========

View File

@ -2,9 +2,15 @@
Django 3.0.4 release notes 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 Bugfixes
======== ========

View File

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

View File

@ -434,6 +434,37 @@ class DistanceFunctionsTests(FuncTestMixin, TestCase):
).filter(d=D(m=1)) ).filter(d=D(m=1))
self.assertTrue(qs.exists()) 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") @skipIfDBFeature("supports_distance_geodetic")
@skipUnlessDBFeature("has_Distance_function") @skipUnlessDBFeature("has_Distance_function")
def test_distance_function_raw_result_d_lookup(self): def test_distance_function_raw_result_d_lookup(self):

View File

@ -9,7 +9,7 @@ from django.contrib.gis.geos import (
MultiPoint, MultiPolygon, Point, Polygon, fromstr, MultiPoint, MultiPolygon, Point, Polygon, fromstr,
) )
from django.core.management import call_command 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.db.models import F, OuterRef, Subquery
from django.test import TestCase, skipUnlessDBFeature from django.test import TestCase, skipUnlessDBFeature
@ -594,6 +594,42 @@ class GeoQuerySetTest(TestCase):
qs = City.objects.filter(name='NotACity') qs = City.objects.filter(name='NotACity')
self.assertIsNone(qs.aggregate(Union('point'))['point__union']) 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): def test_within_subquery(self):
""" """
Using a queryset inside a geo lookup is working (using a subquery) Using a queryset inside a geo lookup is working (using a subquery)