diff --git a/django/contrib/postgres/fields/hstore.py b/django/contrib/postgres/fields/hstore.py index de2757b6936..52cdd215078 100644 --- a/django/contrib/postgres/fields/hstore.py +++ b/django/contrib/postgres/fields/hstore.py @@ -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: diff --git a/django/contrib/postgres/fields/jsonb.py b/django/contrib/postgres/fields/jsonb.py index 966e8f11418..be98ff2d48d 100644 --- a/django/contrib/postgres/fields/jsonb.py +++ b/django/contrib/postgres/fields/jsonb.py @@ -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): diff --git a/docs/releases/1.11.23.txt b/docs/releases/1.11.23.txt index c95ffd9a503..03b33ebf630 100644 --- a/docs/releases/1.11.23.txt +++ b/docs/releases/1.11.23.txt @@ -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 ` for +:class:`~django.contrib.postgres.fields.JSONField` and :lookup:`key lookups +` 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()``. diff --git a/docs/releases/2.1.11.txt b/docs/releases/2.1.11.txt index 9cae1e6f2ef..0de4175b5f4 100644 --- a/docs/releases/2.1.11.txt +++ b/docs/releases/2.1.11.txt @@ -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 ` for +:class:`~django.contrib.postgres.fields.JSONField` and :lookup:`key lookups +` 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()``. diff --git a/docs/releases/2.2.4.txt b/docs/releases/2.2.4.txt index c9653736775..3aac51869ca 100644 --- a/docs/releases/2.2.4.txt +++ b/docs/releases/2.2.4.txt @@ -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 ` for +:class:`~django.contrib.postgres.fields.JSONField` and :lookup:`key lookups +` 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 ======== diff --git a/tests/postgres_tests/test_hstore.py b/tests/postgres_tests/test_hstore.py index 4c8f787de6f..42295172099 100644 --- a/tests/postgres_tests/test_hstore.py +++ b/tests/postgres_tests/test_hstore.py @@ -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): diff --git a/tests/postgres_tests/test_json.py b/tests/postgres_tests/test_json.py index 4a67d177a83..1360bc85dcb 100644 --- a/tests/postgres_tests/test_json.py +++ b/tests/postgres_tests/test_json.py @@ -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):