티스토리 수익 글 보기

티스토리 수익 글 보기

[2.2.x] Fixed CVE-2022-28346 — Protected QuerySet.annotate(), aggreg… · django/django@2c09e68 · GitHub
Skip to content

Commit 2c09e68

Browse files
felixxmfelixxm
committed
[2.2.x] Fixed CVE-2022-28346 — Protected QuerySet.annotate(), aggregate(), and extra() against SQL injection in column aliases.
Thanks Splunk team: Preston Elder, Jacob Davis, Jacob Moore, Matt Hanson, David Briggs, and a security researcher: Danylo Dmytriiev (DDV_UA) for the report. Backport of 93cae5c from main.
1 parent 8352b98 commit 2c09e68

File tree

6 files changed

+83
–0
lines changed

6 files changed

+83
–0
lines changed

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
"""
99
import difflib
1010
import functools
11+
import re
1112
from collections import Counter, OrderedDict, namedtuple
1213
from collections.abc import Iterator, Mapping
1314
from itertools import chain, count, product
@@ -40,6 +41,10 @@
4041

4142
__all__ = ['Query', 'RawQuery']
4243

44+
# Quotation marks ('"`[]), whitespace characters, semicolons, or inline
45+
# SQL comments are forbidden in column aliases.
46+
FORBIDDEN_ALIAS_PATTERN = re.compile(r"['`\"\]\[;\s]|--|/\*|\*/")
47+
4348

4449
def get_field_names_from_opts(opts):
4550
return set(chain.from_iterable(
@@ -994,8 +999,16 @@ def join_parent_model(self, opts, model, alias, seen):
994999
alias = seen[int_model] = join_info.joins[-1]
9951000
return alias or seen[None]
9961001

1002+
def check_alias(self, alias):
1003+
if FORBIDDEN_ALIAS_PATTERN.search(alias):
1004+
raise ValueError(
1005+
"Column aliases cannot contain whitespace characters, quotation marks, "
1006+
"semicolons, or SQL comments."
1007+
)
1008+
9971009
def add_annotation(self, annotation, alias, is_summary=False):
9981010
"""Add a single annotation expression to the Query."""
1011+
self.check_alias(alias)
9991012
annotation = annotation.resolve_expression(self, allow_joins=True, reuse=None,
10001013
summarize=is_summary)
10011014
self.append_annotation_mask([alias])
@@ -1873,6 +1886,7 @@ def add_extra(self, select, select_params, where, params, tables, order_by):
18731886
else:
18741887
param_iter = iter([])
18751888
for name, entry in select.items():
1889+
self.check_alias(name)
18761890
entry = str(entry)
18771891
entry_params = []
18781892
pos = entry.find("%s")

‎docs/releases/2.2.28.txt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,11 @@ Django 2.2.28 release notes
55
*April 11, 2022*
66

77
Django 2.2.28 fixes two security issues with severity "high" in 2.2.27.
8+
9+
CVE-2022-28346: Potential SQL injection in ``QuerySet.annotate()``, ``aggregate()``, and ``extra()``
10+
====================================================================================================
11+
12+
:meth:`.QuerySet.annotate`, :meth:`~.QuerySet.aggregate`, and
13+
:meth:`~.QuerySet.extra` methods were subject to SQL injection in column
14+
aliases, using a suitably crafted dictionary, with dictionary expansion, as the
15+
``**kwargs`` passed to these methods.

‎tests/aggregation/tests.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,3 +1114,12 @@ def test_arguments_must_be_expressions(self):
11141114
Book.objects.aggregate(is_book=True)
11151115
with self.assertRaisesMessage(TypeError, msg % ', '.join([str(FloatField()), 'True'])):
11161116
Book.objects.aggregate(FloatField(), Avg('price'), is_book=True)
1117+
1118+
def test_alias_sql_injection(self):
1119+
crafted_alias = """injected_name" from "aggregation_author"; --"""
1120+
msg = (
1121+
"Column aliases cannot contain whitespace characters, quotation marks, "
1122+
"semicolons, or SQL comments."
1123+
)
1124+
with self.assertRaisesMessage(ValueError, msg):
1125+
Author.objects.aggregate(**{crafted_alias: Avg("age")})

‎tests/annotations/tests.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,3 +598,37 @@ def test_annotation_filter_with_subquery(self):
598598
total_books=Subquery(long_books_qs, output_field=IntegerField()),
599599
).values('name')
600600
self.assertCountEqual(publisher_books_qs, [{'name': 'Sams'}, {'name': 'Morgan Kaufmann'}])
601+
602+
def test_alias_sql_injection(self):
603+
crafted_alias = """injected_name" from "annotations_book"; --"""
604+
msg = (
605+
"Column aliases cannot contain whitespace characters, quotation marks, "
606+
"semicolons, or SQL comments."
607+
)
608+
with self.assertRaisesMessage(ValueError, msg):
609+
Book.objects.annotate(**{crafted_alias: Value(1)})
610+
611+
def test_alias_forbidden_chars(self):
612+
tests = [
613+
'al"ias',
614+
"a'lias",
615+
"ali`as",
616+
"alia s",
617+
"alias\t",
618+
"ali\nas",
619+
"alias--",
620+
"ali/*as",
621+
"alias*/",
622+
"alias;",
623+
# [] are used by MSSQL.
624+
"alias[",
625+
"alias]",
626+
]
627+
msg = (
628+
"Column aliases cannot contain whitespace characters, quotation marks, "
629+
"semicolons, or SQL comments."
630+
)
631+
for crafted_alias in tests:
632+
with self.subTest(crafted_alias):
633+
with self.assertRaisesMessage(ValueError, msg):
634+
Book.objects.annotate(**{crafted_alias: Value(1)})

‎tests/expressions/test_queryset_values.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,15 @@ def test_values_expression(self):
2727
[{'salary': 10}, {'salary': 20}, {'salary': 30}],
2828
)
2929

30+
def test_values_expression_alias_sql_injection(self):
31+
crafted_alias = """injected_name" from "expressions_company"; --"""
32+
msg = (
33+
"Column aliases cannot contain whitespace characters, quotation marks, "
34+
"semicolons, or SQL comments."
35+
)
36+
with self.assertRaisesMessage(ValueError, msg):
37+
Company.objects.values(**{crafted_alias: F("ceo__salary")})
38+
3039
def test_values_expression_group_by(self):
3140
# values() applies annotate() first, so values selected are grouped by
3241
# id, not firstname.

‎tests/queries/tests.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1737,6 +1737,15 @@ def test_extra_select_literal_percent_s(self):
17371737
'bar %s'
17381738
)
17391739

1740+
def test_extra_select_alias_sql_injection(self):
1741+
crafted_alias = """injected_name" from "queries_note"; --"""
1742+
msg = (
1743+
"Column aliases cannot contain whitespace characters, quotation marks, "
1744+
"semicolons, or SQL comments."
1745+
)
1746+
with self.assertRaisesMessage(ValueError, msg):
1747+
Note.objects.extra(select={crafted_alias: "1"})
1748+
17401749

17411750
class SelectRelatedTests(TestCase):
17421751
def test_tickets_3045_3288(self):

0 commit comments

Comments
 (0)