8000 [1.11.x] Fixed CVE-2020-9402 -- Properly escaped tolerance parameter … · django/django@02d97f3 · GitHub
[go: up one dir, main page]
Skip to content

Commit 02d97f3

Browse files
felixxmfelixxm
committed
[1.11.x] Fixed CVE-2020-9402 -- Properly escaped tolerance parameter in GIS functions and aggregates on Oracle.
Thanks to Norbert Szetei for the report.
1 parent e643833 commit 02d97f3

File tree

6 files changed

+97
-7
lines changed

6 files changed

+97
-7
lines changed

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from django.contrib.gis.db.models.fields import ExtentField
2+
from django.db.models import Value
23
from django.db.models.aggregates import Aggregate
34

45
__all__ = ['Collect', 'Extent', 'Extent3D', 'MakeLine', 'Union']
@@ -16,11 +17,14 @@ def as_sql(self, compiler, connection):
1617
return super(GeoAggregate, self).as_sql(compiler, connection)
1718

1819
def as_oracle(self, compiler, connection):
19-
if not hasattr(self, 'tolerance'):
20-
self.tolerance = 0.05
21-
self.extra['tolerance'] = self.tolerance
2220
if not self.is_extent:
23-
self.template = '%(function)s(SDOAGGRTYPE(%(expressions)s,%(tolerance)s))'
21+
tolerance = self.extra.get('tolerance') or getattr(self, 'tolerance', 0.05)
22+
clone = self.copy()
23+
expressions = clone.get_source_expressions()
24+
expressions.append(Value(tolerance))
25+
clone.set_source_expressions(expressions)
26+
clone.template = '%(function)s(SDOAGGRTYPE(%(expressions)s))'
27+
return clone.as_sql(compiler, connection)
2428
return self.as_sql(compiler, connection)
2529

2630
def resolve_expression(self, query=None, allow_joins=True, reuse=None, summarize=False, for_save=False):

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,11 @@ class OracleToleranceMixin(object):
117117
tolerance = 0.05
118118

119119
def as_oracle(self, compiler, connection):
120-
tol = self.extra.get('tolerance', self.tolerance)
120+
tol = self._handle_param(
121+
self.extra.get('tolerance', self.tolerance),
122+
'tolerance',
123+
NUMERIC_TYPES,
124+
)
121125
self.template = "%%(function)s(%%(expressions)s, %s)" % tol
122126
return super(OracleToleranceMixin, self).as_sql(compiler, connection)
123127

‎docs/releases/1.11.29.txt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
============================
2+
Django 1.11.29 release notes
3+
============================
4+
5+
*March 4, 2020*
6+
7+
Django 1.11.29 fixes a security issue in 1.11.29.
8+
9+
CVE-2020-9402: Potential SQL injection via ``tolerance`` parameter in GIS functions and aggregates on Oracle
10+
============================================================================================================
11+
12+
GIS functions and aggregates on Oracle were subject to SQL injection,
13+
using a suitably crafted ``tolerance``.

‎docs/releases/index.txt‎

Lines changed: 1 addition & 0 deletions
E18F
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ versions of the documentation contain the release notes for any later releases.
2626
.. toctree::
2727
:maxdepth: 1
2828

29+
1.11.29
2930
1.11.28
3031
1.11.27
3132
1.11.26

‎tests/gis_tests/distapp/tests.py‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from __future__ import unicode_literals
22

3-
from unittest import skipIf
3+
from unittest import skipIf, skipUnless
44

55
from django.contrib.gis.db.models.functions import (
66
Area, Distance, Length, Perimeter, Transform,
@@ -588,6 +588,37 @@ def test_distance_geodetic_spheroid(self):
588588
for i, c in enumerate(qs):
589589
self.assertAlmostEqual(sphere_distances[i], c.distance.m, tol)
590590

591+
@skipUnless(
592+
connection.vendor == 'oracle',
593+
'Oracle supports tolerance paremeter.',
594+
)
595+
def test_distance_function_tolerance_escaping(self):
596+
qs = AustraliaCity.objects.annotate(
597+
d=Distance(
598+
'point',
599+
Point(0, 0, srid=3857),
600+
tolerance='0.05) = 1 OR 1=1 OR (1+1',
601+
),
602+
).filter(d=1).values('pk')
603+
msg = 'The tolerance parameter has the wrong type'
604+
with self.assertRaisesMessage(TypeError, msg):
605+
qs.exists()
606+
607+
@skipUnless(
608+
connection.vendor == 'oracle',
609+
'Oracle supports tolerance paremeter.',
610+
)
611+
def test_distance_function_tolerance(self):
612+
# Tolerance is greater than distance.
613+
qs = AustraliaCity.objects.annotate(
614+
d=Distance(
615+
'point',
616+
Point(151.23, -33.95, srid=4326),
617+
tolerance=340.7,
618+
),
619+
).filter(d=0).values('pk')
620+
self.assertIs(qs.exists(), True)
621+
591622
@no_oracle # Oracle already handles geographic distance calculation.
592623
@skipUnlessDBFeature("has_Distance_function", 'has_Transform_function')
593624
def test_distance_transform(self):

‎tests/gis_tests/geoapp/tests.py‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import re
44
import tempfile
5+
import unittest
56

67
from django.contrib.gis import gdal
78
from django.contrib.gis.db.models import Extent, MakeLine, Union
@@ -10,7 +11,7 @@
1011
MultiPoint, MultiPolygon, Point, Polygon, fromstr,
1112
)
1213
from django.core.management import call_command
13-
from django.db import connection
14+
from django.db import DatabaseError, connection
1415
from django.test import TestCase, ignore_warnings, skipUnlessDBFeature
1516
from django.utils import six
1617
from django.utils.deprecation import RemovedInDjango20Warning
@@ -881,6 +882,42 @@ def test_unionagg(self):
881882
qs = City.objects.filter(name='NotACity')
882883
self.assertIsNone(qs.aggregate(Union('point'))['point__union'])
883884

885+
@unittest.skipUnless(
886+
connection.vendor == 'oracle',
887+
'Oracle supports tolerance paremeter.',
888+
)
889+
def test_unionagg_tolerance(self):
890+
City.objects.create(
891+
point=fromstr('POINT(-96.467222 32.751389)', srid=4326),
892+
name='Forney',
893+
)
894+
tx = Country.objects.get(name='Texas').mpoly
895+
# Tolerance is greater than distance between Forney and Dallas, that's
896+
# why Dallas is ignored.
897+
forney_houston = GEOSGeometry(
898+
'MULTIPOINT(-95.363151 29.763374, -96.467222 32.751389)',
899+
srid=4326,
900+
)
901+
self.assertIs(
902+
forney_houston.equals(
903+
City.objects.filter(point__within=tx).aggregate(
904+
Union('point', tolerance=32000),
905+
)['point__union'],
906+
),
907+
True,
908+
)
909+
910+
@unittest.skipUnless(
911+
connection.vendor == 'oracle',
912+
'Oracle supports tolerance paremeter.',
913+
)
914+
def test_unionagg_tolerance_escaping(self):
915+
tx = Country.objects.get(name='Texas').mpoly
916+
with self.assertRaises(DatabaseError):
917+
City.objects.filter(point__within=tx).aggregate(
918+
Union('point', tolerance='0.05))), (((1'),
919+
)
920+
884921
def test_within_subquery(self):
885922
"""
886923
Using a queryset inside a geo lookup is working (using a subquery)

0 commit comments

Comments
 (0)
0