Skip to content
Permalink
Browse files

[2.2.x] Fixed CVE-2019-14234 -- Protected JSONField/HStoreField key a…

…nd index lookups against SQL injection.

Thanks to Sage M. Abdullah for the report and initial patch.
Thanks Florian Apolloner for reviews.
  • Loading branch information...
felixxm authored and carltongibson committed Jul 22, 2019
1 parent e34f3c0 commit 4f5b58f5cd3c57fee9972ab074f8dc6895d8f387
@@ -86,7 +86,7 @@ def __init__(self, key_name, *args, **kwargs):

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:
@@ -109,12 +109,10 @@ def as_sql(self, compiler, connection):
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):
@@ -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()``.
@@ -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()``.
@@ -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
========

@@ -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 @@ def test_usage_in_subquery(self):
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):
@@ -5,9 +5,10 @@

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
@@ -322,6 +323,18 @@ def test_regex(self):
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):

0 comments on commit 4f5b58f

Please sign in to comment.
You can’t perform that action at this time.