Fixed CVE-2019-14234 -- Protected JSONField/HStoreField key and index lookups against SQL injection.

Thanks to Sage M. Abdullah for the report and initial patch.
Thanks Florian Apolloner for reviews.
This commit is contained in:
Mariusz Felisiak 2019-07-22 10:45:26 +02:00 committed by Carlton Gibson
parent 4b78420d25
commit 7deeabc7c7
7 changed files with 59 additions and 8 deletions

View File

@ -86,7 +86,7 @@ class KeyTransform(Transform):
def as_sql(self, compiler, connection):
lhs, params = compiler.compile(self.lhs)
return "(%s -> '%s')" % (lhs, self.key_name), params
return '(%s -> %%s)' % lhs, [self.key_name] + params
class KeyTransformFactory:

View File

@ -109,12 +109,10 @@ class KeyTransform(Transform):
if len(key_transforms) > 1:
return "(%s %s %%s)" % (lhs, self.nested_operator), [key_transforms] + params
try:
int(self.key_name)
lookup = int(self.key_name)
except ValueError:
lookup = "'%s'" % self.key_name
else:
lookup = "%s" % self.key_name
return "(%s %s %s)" % (lhs, self.operator, lookup), params
lookup = self.key_name
return '(%s %s %%s)' % (lhs, self.operator), [lookup] + params
class KeyTextTransform(KeyTransform):

View File

@ -36,3 +36,12 @@ Remember that absolutely NO guarantee is provided about the results of
``strip_tags()`` being HTML safe. So NEVER mark safe the result of a
``strip_tags()`` call without escaping it first, for example with
:func:`django.utils.html.escape`.
CVE-2019-14234: SQL injection possibility in key and index lookups for ``JSONField``/``HStoreField``
====================================================================================================
:lookup:`Key and index lookups <jsonfield.key>` for
:class:`~django.contrib.postgres.fields.JSONField` and :lookup:`key lookups
<hstorefield.key>` for :class:`~django.contrib.postgres.fields.HStoreField`
were subject to SQL injection, using a suitably crafted dictionary, with
dictionary expansion, as the ``**kwargs`` passed to ``QuerySet.filter()``.

View File

@ -36,3 +36,12 @@ Remember that absolutely NO guarantee is provided about the results of
``strip_tags()`` being HTML safe. So NEVER mark safe the result of a
``strip_tags()`` call without escaping it first, for example with
:func:`django.utils.html.escape`.
CVE-2019-14234: SQL injection possibility in key and index lookups for ``JSONField``/``HStoreField``
====================================================================================================
:lookup:`Key and index lookups <jsonfield.key>` for
:class:`~django.contrib.postgres.fields.JSONField` and :lookup:`key lookups
<hstorefield.key>` for :class:`~django.contrib.postgres.fields.HStoreField`
were subject to SQL injection, using a suitably crafted dictionary, with
dictionary expansion, as the ``**kwargs`` passed to ``QuerySet.filter()``.

View File

@ -37,6 +37,15 @@ Remember that absolutely NO guarantee is provided about the results of
``strip_tags()`` call without escaping it first, for example with
:func:`django.utils.html.escape`.
CVE-2019-14234: SQL injection possibility in key and index lookups for ``JSONField``/``HStoreField``
====================================================================================================
:lookup:`Key and index lookups <jsonfield.key>` for
:class:`~django.contrib.postgres.fields.JSONField` and :lookup:`key lookups
<hstorefield.key>` for :class:`~django.contrib.postgres.fields.HStoreField`
were subject to SQL injection, using a suitably crafted dictionary, with
dictionary expansion, as the ``**kwargs`` passed to ``QuerySet.filter()``.
Bugfixes
========

View File

@ -1,8 +1,9 @@
import json
from django.core import checks, exceptions, serializers
from django.db import connection
from django.forms import Form
from django.test.utils import isolate_apps
from django.test.utils import CaptureQueriesContext, isolate_apps
from . import PostgreSQLSimpleTestCase, PostgreSQLTestCase
from .models import HStoreModel, PostgreSQLModel
@ -185,6 +186,18 @@ class TestQuerying(PostgreSQLTestCase):
self.objs[:2]
)
def test_key_sql_injection(self):
with CaptureQueriesContext(connection) as queries:
self.assertFalse(
HStoreModel.objects.filter(**{
"field__test' = 'a') OR 1 = 1 OR ('d": 'x',
}).exists()
)
self.assertIn(
"""."field" -> 'test'' = ''a'') OR 1 = 1 OR (''d') = 'x' """,
queries[0]['sql'],
)
@isolate_apps('postgres_tests')
class TestChecks(PostgreSQLSimpleTestCase):

View File

@ -5,9 +5,10 @@ from decimal import Decimal
from django.core import checks, exceptions, serializers
from django.core.serializers.json import DjangoJSONEncoder
from django.db import connection
from django.db.models import Count, Q
from django.forms import CharField, Form, widgets
from django.test.utils import isolate_apps
from django.test.utils import CaptureQueriesContext, isolate_apps
from django.utils.html import escape
from . import PostgreSQLSimpleTestCase, PostgreSQLTestCase
@ -331,6 +332,18 @@ class TestQuerying(PostgreSQLTestCase):
def test_iregex(self):
self.assertTrue(JSONModel.objects.filter(field__foo__iregex=r'^bAr$').exists())
def test_key_sql_injection(self):
with CaptureQueriesContext(connection) as queries:
self.assertFalse(
JSONModel.objects.filter(**{
"""field__test' = '"a"') OR 1 = 1 OR ('d""": 'x',
}).exists()
)
self.assertIn(
"""."field" -> 'test'' = ''"a"'') OR 1 = 1 OR (''d') = '"x"' """,
queries[0]['sql'],
)
@isolate_apps('postgres_tests')
class TestChecks(PostgreSQLSimpleTestCase):