Skip to content

Commit 00b0fc5

Browse files
felixxmfelixxm
committed
[4.0.x] Fixed CVE-2022-28347 -- Protected QuerySet.explain(**options) against SQL injection on PostgreSQL.
Backport of 6723a26 from main.
1 parent 8008288 commit 00b0fc5

File tree

7 files changed

+85
-11
lines changed

7 files changed

+85
-11
lines changed

‎django/db/backends/postgresql/features.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ class DatabaseFeatures(BaseDatabaseFeatures):
5454
only_supports_unbounded_with_preceding_and_following = True
5555
supports_aggregate_filter_clause = True
5656
supported_explain_formats = {"JSON", "TEXT", "XML", "YAML"}
57-
validates_explain_options = False # A query will error on invalid options.
5857
supports_deferrable_unique_constraints = True
5958
has_json_operators = True
6059
json_key_contains_list_matching_requires_list = True

‎django/db/backends/postgresql/operations.py‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,18 @@
88
class DatabaseOperations(BaseDatabaseOperations):
99
cast_char_field_without_max_length = "varchar"
1010
explain_prefix = "EXPLAIN"
11+
explain_options = frozenset(
12+
[
13+
"ANALYZE",
14+
"BUFFERS",
15+
"COSTS",
16+
"SETTINGS",
17+
"SUMMARY",
18+
"TIMING",
19+
"VERBOSE",
20+
"WAL",
21+
]
22+
)
1123
cast_data_types = {
1224
"AutoField": "integer",
1325
"BigAutoField": "bigint",
@@ -288,17 +300,20 @@ def subtract_temporals(self, internal_type, lhs, rhs):
288300
return super().subtract_temporals(internal_type, lhs, rhs)
289301

290302
def explain_query_prefix(self, format=None, **options):
291-
prefix = super().explain_query_prefix(format)
292303
extra = {}
304+
# Normalize options.
305+
if options:
306+
options = {
307+
name.upper(): "true" if value else "false"
308+
for name, value in options.items()
309+
}
310+
for valid_option in self.explain_options:
311+
value = options.pop(valid_option, None)
312+
if value is not None:
313+
extra[valid_option.upper()] = value
314+
prefix = super().explain_query_prefix(format, **options)
293315
if format:
294316
extra["FORMAT"] = format
295-
if options:
296-
extra.update(
297-
{
298-
name.upper(): "true" if value else "false"
299-
for name, value in options.items()
300-
}
301-
)
302317
if extra:
303318
prefix += " (%s)" % ", ".join("%s %s" % i for i in extra.items())
304319
return prefix

‎django/db/models/sql/query.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@
4949
# SQL comments are forbidden in column aliases.
5050
FORBIDDEN_ALIAS_PATTERN = _lazy_re_compile(r"['`\"\]\[;\s]|--|/\*|\*/")
5151

52+
# Inspired from
53+
# https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS
54+
EXPLAIN_OPTIONS_PATTERN = _lazy_re_compile(r"[\w\-]+")
55+
5256

5357
def get_field_names_from_opts(opts):
5458
return set(
@@ -586,6 +590,12 @@ def has_results(self, using):
586590

587591
def explain(self, using, format=None, **options):
588592
q = self.clone()
593+
for option_name in options:
594+
if (
595+
not EXPLAIN_OPTIONS_PATTERN.fullmatch(option_name)
596+
or "--" in option_name
597+
):
598+
raise ValueError(f"Invalid option name: {option_name!r}.")
589599
q.explain_info = ExplainInfo(format, options)
590600
compiler = q.get_compiler(using=using)
591601
return "\n".join(compiler.explain_query())

‎docs/releases/2.2.28.txt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,10 @@ CVE-2022-28346: Potential SQL injection in ``QuerySet.annotate()``, ``aggregate(
1313
:meth:`~.QuerySet.extra` methods were subject to SQL injection in column
1414
aliases, using a suitably crafted dictionary, with dictionary expansion, as the
1515
``**kwargs`` passed to these methods.
16+
17+
CVE-2022-28347: Potential SQL injection via ``QuerySet.explain(**options)`` on PostgreSQL
18+
=========================================================================================
19+
20+
:meth:`.QuerySet.explain` method was subject to SQL injection in option names,
21+
using a suitably crafted dictionary, with dictionary expansion, as the
22+
``**options`` argument.

‎docs/releases/3.2.13.txt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@ CVE-2022-28346: Potential SQL injection in ``QuerySet.annotate()``, ``aggregate(
1515
aliases, using a suitably crafted dictionary, with dictionary expansion, as the
1616
``**kwargs`` passed to these methods.
1717

18+
CVE-2022-28347: Potential SQL injection via ``QuerySet.explain(**options)`` on PostgreSQL
19+
=========================================================================================
20+
21+
:meth:`.QuerySet.explain` method was subject to SQL injection in option names,
22+
using a suitably crafted dictionary, with dictionary expansion, as the
23+
``**options`` argument.
24+
1825
Bugfixes
1926
========
2027

‎docs/releases/4.0.4.txt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@ CVE-2022-28346: Potential SQL injection in ``QuerySet.annotate()``, ``aggregate(
1515
aliases, using a suitably crafted dictionary, with dictionary expansion, as the
1616
``**kwargs`` passed to these methods.
1717

18+
CVE-2022-28347: Potential SQL injection via ``QuerySet.explain(**options)`` on PostgreSQL
19+
=========================================================================================
20+
21+
:meth:`.QuerySet.explain` method was subject to SQL injection in option names,
22+
using a suitably crafted dictionary, with dictionary expansion, as the
23+
``**options`` argument.
24+
1825
Bugfixes
1926
========
2027

‎tests/queries/test_explain.py‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ def test_basic(self):
5959

6060
@skipUnlessDBFeature("validates_explain_options")
6161
def test_unknown_options(self):
62-
with self.assertRaisesMessage(ValueError, "Unknown options: test, test2"):
63-
Tag.objects.all().explain(test=1, test2=1)
62+
with self.assertRaisesMessage(ValueError, "Unknown options: TEST, TEST2"):
63+
Tag.objects.all().explain(**{"TEST": 1, "TEST2": 1})
6464

6565
def test_unknown_format(self):
6666
msg = "DOES NOT EXIST is not a recognized format."
@@ -94,6 +94,35 @@ def test_postgres_options(self):
9494
option = "{} {}".format(name.upper(), "true" if value else "false")
9595
self.assertIn(option, captured_queries[0]["sql"])
9696

97+
def test_option_sql_injection(self):
98+
qs = Tag.objects.filter(name="test")
99+
options = {"SUMMARY true) SELECT 1; --": True}
100+
msg = "Invalid option name: 'SUMMARY true) SELECT 1; --'"
101+
with self.assertRaisesMessage(ValueError, msg):
102+
qs.explain(**options)
103+
104+
def test_invalid_option_names(self):
105+
qs = Tag.objects.filter(name="test")
106+
tests = [
107+
'opt"ion',
108+
"o'ption",
109+
"op`tion",
110+
"opti on",
111+
"option--",
112+
"optio\tn",
113+
"o\nption",
114+
"option;",
115+
"你 好",
116+
# [] are used by MSSQL.
117+
"option[",
118+
"option]",
119+
]
120+
for invalid_option in tests:
121+
with self.subTest(invalid_option):
122+
msg = f"Invalid option name: {invalid_option!r}"
123+
with self.assertRaisesMessage(ValueError, msg):
124+
qs.explain(**{invalid_option: True})
125+
97126
@unittest.skipUnless(connection.vendor == "mysql", "MySQL specific")
98127
def test_mysql_text_to_traditional(self):
99128
# Ensure these cached properties are initialized to prevent queries for

0 commit comments

Comments
 (0)