8000 [3.0.x] Fixed CVE-2020-9402 -- Properly escaped tolerance parameter i… · django/django@26a5cf8 · GitHub
[go: up one dir, main page]

Skip to content

Commit 26a5cf8

Browse files
committed
[3.0.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 c5cfaad commit 26a5cf8

File tree

8 files changed

+117
-14
lines changed

8 files changed

+117
-14
lines changed

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from django.contrib.gis.db.models.fields import (
22
ExtentField, GeometryCollectionField, GeometryField, LineStringField,
33
)
4+
from django.db.models import Value
45
from django.db.models.aggregates import Aggregate
56
from django.utils.functional import cached_property
67

@@ -27,9 +28,16 @@ def as_sql(self, compiler, connection, function=None, **extra_context):
2728
)
2829

2930
def as_oracle(self, compiler, connection, **extra_context):
30-
tolerance = self.extra.get('tolerance') or getattr(self, 'tolerance', 0.05)
31-
template = None if self.is_extent else '%(function)s(SDOAGGRTYPE(%(expressions)s,%(tolerance)s))'
32-
return self.as_sql(compiler, connection, template=template, tolerance=tolerance, **extra_context)
31+
if not self.is_extent:
32+
tolerance = self.extra.get('tolerance') or getattr(self, 'tolerance', 0.05)
33+
clone = self.copy()
34+
clone.set_source_expressions([
35+
*self.get_source_expressions(),
36+
Value(tolerance),
37+
])
38+
template = '%(function)s(SDOAGGRTYPE(%(expressions)s))'
39+
return clone.as_sql(compiler, connection, template=template, **extra_context)
40+
return self.as_sql(compiler, connection, **extra_context)
3341

3442
def resolve_expression(self, query=None, allow_joins=True, reuse=None, summarize=False, for_save=False):
3543
c = super().resolve_expression(query, allow_joins, reuse, summarize, for_save)

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,14 @@ class OracleToleranceMixin:
111111
tolerance = 0.05
112112

113113
def as_oracle(self, compiler, connection, **extra_context):
114-
tol = self.extra.get('tolerance', self.tolerance)
115-
return self.as_sql(
116-
compiler, connection,
117-
template="%%(function)s(%%(expressions)s, %s)" % tol,
118-
**extra_context
119-
)
114+
tolerance = Value(self._handle_param(
115+
self.extra.get('tolerance', self.tolerance),
116+
'tolerance',
117+
NUMERIC_TYPES,
118+
))
119+
clone = self.copy()
120+
clone.set_source_expressions([*self.get_source_expressions(), tolerance])
121+
return clone.as_sql(compiler, connection, **extra_context)
120122

121123

122124
class Area(OracleToleranceMixin, GeoFunc):

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/2.2.11.txt

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,15 @@
22
Django 2.2.11 release notes
33
===========================
44

5-
*Expected March 2, 2020*
5+
*March 4, 2020*
66

7-
Django 2.2.11 fixes a data loss bug in 2.2.10.
7+
Django 2.2.11 fixes a security issue and a data loss bug in 2.2.10.
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``.
814

915
Bugfixes
1016
========

docs/releases/3.0.4.txt

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,15 @@
22
Django 3.0.4 release notes
33
==========================
44

5-
*Expected March 2, 2020*
5+
*March 4, 2020*
66

7-
Django 3.0.4 fixes several bugs in 3.0.3.
7+
Django 3.0.4 fixes a security issue and several bugs in 3.0.3.
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``.
814

915
Bugfixes
1016
========

docs/releases/index.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ versions of the documentation contain the release notes for any later releases.
9696
.. toctree::
9797
:maxdepth: 1
9898

99+
1.11.29
99100
1.11.28
100101
1.11.27
101102
1.11.26

tests/gis_tests/distapp/tests.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,37 @@ def test_distance_function_d_lookup(self):
434434
).filter(d=D(m=1))
435435
self.assertTrue(qs.exists())
436436

437+
@unittest.skipUnless(
438+
connection.vendor == 'oracle',
439+
'Oracle supports tolerance paremeter.',
440+
)
441+
def test_distance_function_tolerance_escaping(self):
442+
qs = Interstate.objects.annotate(
443+
d=Distance(
444+
Point(500, 500, srid=3857),
445+
Point(0, 0, srid=3857),
446+
tolerance='0.05) = 1 OR 1=1 OR (1+1',
447+
),
448+
).filter(d=D(m=1)).values('pk')
449+
msg = 'The tolerance parameter has the wrong type'
450+
with self.assertRaisesMessage(TypeError, msg):
451+
qs.exists()
452+
453+
@unittest.skipUnless(
454+
connection.vendor == 'oracle',
455+
'Oracle supports tolerance paremeter.',
456+
)
457+
def test_distance_function_tolerance(self):
458+
# Tolerance is greater than distance.
459+
qs = Interstate.objects.annotate(
460+
d=Distance(
461+
Point(0, 0, srid=3857),
462+
Point(1, 1, srid=3857),
463+
tolerance=1.5,
464+
),
465+
).filter(d=0).values('pk')
466+
self.assertIs(qs.exists(), True)
467+
437468
@skipIfDBFeature("supports_distance_geodetic")
438469
@skipUnlessDBFeature("has_Distance_function")
439470
def test_distance_function_raw_result_d_lookup(self):

tests/gis_tests/geoapp/tests.py

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
MultiPoint, MultiPolygon, Point, Polygon, fromstr,
1010
)
1111
from django.core.management import call_command
12-
from django.db import NotSupportedError, connection
12+
from django.db import DatabaseError, NotSupportedError, connection
1313
from django.test import TestCase, skipUnlessDBFeature
1414

1515
from ..utils import (
@@ -564,6 +564,42 @@ def test_unionagg(self):
564564
qs = City.objects.filter(name='NotACity')
565565
self.assertIsNone(qs.aggregate(Union('point'))['point__union'])
566566

567+
@unittest.skipUnless(
568+
connection.vendor == 'oracle',
569+
'Oracle supports tolerance paremeter.',
570+
)
571+
def test_unionagg_tolerance(self):
572+
City.objects.create(
573+
point=fromstr('POINT(-96.467222 32.751389)', srid=4326),
574+
name='Forney',
575+
)
576+
tx = Country.objects.get(name='Texas').mpoly
577+
# Tolerance is greater than distance between Forney and Dallas, that's
578+
# why Dallas is ignored.
579+
forney_houston = GEOSGeometry(
580+
'MULTIPOINT(-95.363151 29.763374, -96.467222 32.751389)',
581+
srid=4326,
582+
)
583+
self.assertIs(
584+
forney_houston.equals(
585+
City.objects.filter(point__within=tx).aggregate(
586+
Union('point', tolerance=32000),
587+
)['point__union'],
588+
),
589+
True,
590+
)
591+
592+
@unittest.skipUnless(
593+
connection.vendor == 'oracle',
594+
'Oracle supports tolerance paremeter.',
595+
)
596+
def test_unionagg_tolerance_escaping(self):
597+
tx = Country.objects.get(name='Texas').mpoly
598+
with self.assertRaises(DatabaseError):
599+
City.objects.filter(point__within=tx).aggregate(
600+
Union('point', tolerance='0.05))), (((1'),
601+
)
602+
567603
def test_within_subquery(self):
568604
"""
569605
Using a queryset inside a geo lookup is working (using a subquery)

0 commit comments

Comments
 (0)
0