1
0
mirror of https://github.com/django/django.git synced 2024-12-23 01:25:58 +00:00

[4.2.x] Fixed CVE-2024-53908 -- Prevented SQL injections in direct HasKeyLookup usage on Oracle.

Thanks Seokchan Yoon for the report, and Mariusz Felisiak and Sarah
Boyce for the reviews.
This commit is contained in:
Simon Charette 2024-11-08 21:27:31 -05:00 committed by Sarah Boyce
parent 790eb058b0
commit 7376bcbf50
3 changed files with 53 additions and 18 deletions

View File

@ -216,20 +216,18 @@ class HasKeyLookup(PostgresOperatorLookup):
# Compile the final key without interpreting ints as array elements.
return ".%s" % json.dumps(key_transform)
def as_sql(self, compiler, connection, template=None):
def _as_sql_parts(self, compiler, connection):
# Process JSON path from the left-hand side.
if isinstance(self.lhs, KeyTransform):
lhs, lhs_params, lhs_key_transforms = self.lhs.preprocess_lhs(
lhs_sql, lhs_params, lhs_key_transforms = self.lhs.preprocess_lhs(
compiler, connection
)
lhs_json_path = compile_json_path(lhs_key_transforms)
else:
lhs, lhs_params = self.process_lhs(compiler, connection)
lhs_sql, lhs_params = self.process_lhs(compiler, connection)
lhs_json_path = "$"
sql = template % lhs
# Process JSON path from the right-hand side.
rhs = self.rhs
rhs_params = []
if not isinstance(rhs, (list, tuple)):
rhs = [rhs]
for key in rhs:
@ -240,24 +238,43 @@ class HasKeyLookup(PostgresOperatorLookup):
*rhs_key_transforms, final_key = rhs_key_transforms
rhs_json_path = compile_json_path(rhs_key_transforms, include_root=False)
rhs_json_path += self.compile_json_path_final_key(final_key)
rhs_params.append(lhs_json_path + rhs_json_path)
yield lhs_sql, lhs_params, lhs_json_path + rhs_json_path
def _combine_sql_parts(self, parts):
# Add condition for each key.
if self.logical_operator:
sql = "(%s)" % self.logical_operator.join([sql] * len(rhs_params))
return sql, tuple(lhs_params) + tuple(rhs_params)
return "(%s)" % self.logical_operator.join(parts)
return "".join(parts)
def as_sql(self, compiler, connection, template=None):
sql_parts = []
params = []
for lhs_sql, lhs_params, rhs_json_path in self._as_sql_parts(
compiler, connection
):
sql_parts.append(template % (lhs_sql, "%s"))
params.extend(lhs_params + [rhs_json_path])
return self._combine_sql_parts(sql_parts), tuple(params)
def as_mysql(self, compiler, connection):
return self.as_sql(
compiler, connection, template="JSON_CONTAINS_PATH(%s, 'one', %%s)"
compiler, connection, template="JSON_CONTAINS_PATH(%s, 'one', %s)"
)
def as_oracle(self, compiler, connection):
sql, params = self.as_sql(
compiler, connection, template="JSON_EXISTS(%s, '%%s')"
)
# Add paths directly into SQL because path expressions cannot be passed
# as bind variables on Oracle.
return sql % tuple(params), []
template = "JSON_EXISTS(%s, '%s')"
sql_parts = []
params = []
for lhs_sql, lhs_params, rhs_json_path in self._as_sql_parts(
compiler, connection
):
# Add right-hand-side directly into SQL because it cannot be passed
# as bind variables to JSON_EXISTS. It might result in invalid
# queries but it is assumed that it cannot be evaded because the
# path is JSON serialized.
sql_parts.append(template % (lhs_sql, rhs_json_path))
params.extend(lhs_params)
return self._combine_sql_parts(sql_parts), tuple(params)
def as_postgresql(self, compiler, connection):
if isinstance(self.rhs, KeyTransform):
@ -269,7 +286,7 @@ class HasKeyLookup(PostgresOperatorLookup):
def as_sqlite(self, compiler, connection):
return self.as_sql(
compiler, connection, template="JSON_TYPE(%s, %%s) IS NOT NULL"
compiler, connection, template="JSON_TYPE(%s, %s) IS NOT NULL"
)
@ -467,9 +484,9 @@ class KeyTransformIsNull(lookups.IsNull):
return "(NOT %s OR %s IS NULL)" % (sql, lhs), tuple(params) + tuple(lhs_params)
def as_sqlite(self, compiler, connection):
template = "JSON_TYPE(%s, %%s) IS NULL"
template = "JSON_TYPE(%s, %s) IS NULL"
if not self.rhs:
template = "JSON_TYPE(%s, %%s) IS NOT NULL"
template = "JSON_TYPE(%s, %s) IS NOT NULL"
return HasKeyOrArrayIndex(self.lhs.lhs, self.lhs.key_name).as_sql(
compiler,
connection,

View File

@ -22,3 +22,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-2024-53908: Potential SQL injection via ``HasKey(lhs, rhs)`` on Oracle
==========================================================================
Direct usage of the ``django.db.models.fields.json.HasKey`` lookup on Oracle
was subject to SQL injection if untrusted data was used as a ``lhs`` value.
Applications that use the :lookup:`has_key <jsonfield.has_key>` lookup through
the ``__`` syntax are unaffected.

View File

@ -29,6 +29,7 @@ from django.db.models import (
from django.db.models.expressions import RawSQL
from django.db.models.fields.json import (
KT,
HasKey,
KeyTextTransform,
KeyTransform,
KeyTransformFactory,
@ -607,6 +608,14 @@ class TestQuerying(TestCase):
[expected],
)
def test_has_key_literal_lookup(self):
self.assertSequenceEqual(
NullableJSONModel.objects.filter(
HasKey(Value({"foo": "bar"}, JSONField()), "foo")
).order_by("id"),
self.objs,
)
def test_has_key_list(self):
obj = NullableJSONModel.objects.create(value=[{"a": 1}, {"b": "x"}])
tests = [