Skip to content

Commit 29a6c98

Browse files
felixxmfelixxm
committed
[2.2.x] Fixed CVE-2022-28347 -- Protected QuerySet.explain(**options) against SQL injection on PostgreSQL.
Backport of 6723a26 from main.
1 parent 2c09e68 commit 29a6c98

5 files changed

Lines changed: 70 additions & 8 deletions

File tree

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ class DatabaseFeatures(BaseDatabaseFeatures):
5353
supports_over_clause = True
5454
supports_aggregate_filter_clause = True
5555
supported_explain_formats = {'JSON', 'TEXT', 'XML', 'YAML'}
56-
validates_explain_options = False # A query will error on invalid options.
5756

5857
@cached_property
5958
def is_postgresql_9_5(self):

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

Lines changed: 22 additions & 5 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',
@@ -267,15 +279,20 @@ def window_frame_range_start_end(self, start=None, end=None):
267279
return start_, end_
268280

269281
def explain_query_prefix(self, format=None, **options):
270-
prefix = super().explain_query_prefix(format)
271282
extra = {}
272-
if format:
273-
extra['FORMAT'] = format
283+
# Normalize options.
274284
if options:
275-
extra.update({
285+
options = {
276286
name.upper(): 'true' if value else 'false'
277287
for name, value in options.items()
278-
})
288+
}
289+
for valid_option in self.explain_options:
290+
value = options.pop(valid_option, None)
291+
if value is not None:
292+
extra[valid_option.upper()] = value
293+
prefix = super().explain_query_prefix(format, **options)
294+
if format:
295+
extra['FORMAT'] = format
279296
if extra:
280297
prefix += ' (%s)' % ', '.join('%s %s' % i for i in extra.items())
281298
return prefix

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@
4545
# SQL comments are forbidden in column aliases.
4646
FORBIDDEN_ALIAS_PATTERN = re.compile(r"['`\"\]\[;\s]|--|/\*|\*/")
4747

48+
# Inspired from
49+
# https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS
50+
EXPLAIN_OPTIONS_PATTERN = re.compile(r"[\w\-]+")
51+
4852

4953
def get_field_names_from_opts(opts):
5054
return set(chain.from_iterable(
@@ -528,6 +532,12 @@ def has_results(self, using):
528532

529533
def explain(self, using, format=None, **options):
530534
q = self.clone()
535+
for option_name in options:
536+
if (
537+
not EXPLAIN_OPTIONS_PATTERN.fullmatch(option_name) or
538+
"--" in option_name
539+
):
540+
raise ValueError("Invalid option name: '%s'." % option_name)
531541
q.explain_query = True
532542
q.explain_format = format
533543
q.explain_options = options

‎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.

‎tests/queries/test_explain.py‎

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

4242
@skipUnlessDBFeature('validates_explain_options')
4343
def test_unknown_options(self):
44-
with self.assertRaisesMessage(ValueError, 'Unknown options: test, test2'):
45-
Tag.objects.all().explain(test=1, test2=1)
44+
with self.assertRaisesMessage(ValueError, "Unknown options: TEST, TEST2"):
45+
Tag.objects.all().explain(**{"TEST": 1, "TEST2": 1})
4646

4747
def test_unknown_format(self):
4848
msg = 'DOES NOT EXIST is not a recognized format.'
@@ -71,6 +71,35 @@ def test_postgres_options(self):
7171
option = '{} {}'.format(name.upper(), 'true' if value else 'false')
7272
self.assertIn(option, captured_queries[0]['sql'])
7373

74+
def test_option_sql_injection(self):
75+
qs = Tag.objects.filter(name="test")
76+
options = {"SUMMARY true) SELECT 1; --": True}
77+
msg = "Invalid option name: 'SUMMARY true) SELECT 1; --'"
78+
with self.assertRaisesMessage(ValueError, msg):
79+
qs.explain(**options)
80+
81+
def test_invalid_option_names(self):
82+
qs = Tag.objects.filter(name="test")
83+
tests = [
84+
'opt"ion',
85+
"o'ption",
86+
"op`tion",
87+
"opti on",
88+
"option--",
89+
"optio\tn",
90+
"o\nption",
91+
"option;",
92+
"你 好",
93+
# [] are used by MSSQL.
94+
"option[",
95+
"option]",
96+
]
97+
for invalid_option in tests:
98+
with self.subTest(invalid_option):
99+
msg = "Invalid option name: '%s'" % invalid_option
100+
with self.assertRaisesMessage(ValueError, msg):
101+
qs.explain(**{invalid_option: True})
102+
74103
@unittest.skipUnless(connection.vendor == 'mysql', 'MySQL specific')
75104
def test_mysql_text_to_traditional(self):
76105
# Initialize the cached property, if needed, to prevent a query for

0 commit comments

Comments
 (0)