Fixed CVE-2022-28347 -- Protected QuerySet.explain(**options) against SQL injection on PostgreSQL.

This commit is contained in:
Mariusz Felisiak 2022-04-01 13:48:47 +02:00
parent 93cae5cb2f
commit 6723a26e59
7 changed files with 85 additions and 11 deletions

View File

@ -54,7 +54,6 @@ class DatabaseFeatures(BaseDatabaseFeatures):
only_supports_unbounded_with_preceding_and_following = True only_supports_unbounded_with_preceding_and_following = True
supports_aggregate_filter_clause = True supports_aggregate_filter_clause = True
supported_explain_formats = {"JSON", "TEXT", "XML", "YAML"} supported_explain_formats = {"JSON", "TEXT", "XML", "YAML"}
validates_explain_options = False # A query will error on invalid options.
supports_deferrable_unique_constraints = True supports_deferrable_unique_constraints = True
has_json_operators = True has_json_operators = True
json_key_contains_list_matching_requires_list = True json_key_contains_list_matching_requires_list = True

View File

@ -9,6 +9,18 @@ from django.db.models.constants import OnConflict
class DatabaseOperations(BaseDatabaseOperations): class DatabaseOperations(BaseDatabaseOperations):
cast_char_field_without_max_length = "varchar" cast_char_field_without_max_length = "varchar"
explain_prefix = "EXPLAIN" explain_prefix = "EXPLAIN"
explain_options = frozenset(
[
"ANALYZE",
"BUFFERS",
"COSTS",
"SETTINGS",
"SUMMARY",
"TIMING",
"VERBOSE",
"WAL",
]
)
cast_data_types = { cast_data_types = {
"AutoField": "integer", "AutoField": "integer",
"BigAutoField": "bigint", "BigAutoField": "bigint",
@ -298,17 +310,20 @@ class DatabaseOperations(BaseDatabaseOperations):
return super().subtract_temporals(internal_type, lhs, rhs) return super().subtract_temporals(internal_type, lhs, rhs)
def explain_query_prefix(self, format=None, **options): def explain_query_prefix(self, format=None, **options):
prefix = super().explain_query_prefix(format)
extra = {} extra = {}
# Normalize options.
if options:
options = {
name.upper(): "true" if value else "false"
for name, value in options.items()
}
for valid_option in self.explain_options:
value = options.pop(valid_option, None)
if value is not None:
extra[valid_option.upper()] = value
prefix = super().explain_query_prefix(format, **options)
if format: if format:
extra["FORMAT"] = format extra["FORMAT"] = format
if options:
extra.update(
{
name.upper(): "true" if value else "false"
for name, value in options.items()
}
)
if extra: if extra:
prefix += " (%s)" % ", ".join("%s %s" % i for i in extra.items()) prefix += " (%s)" % ", ".join("%s %s" % i for i in extra.items())
return prefix return prefix

View File

@ -49,6 +49,10 @@ __all__ = ["Query", "RawQuery"]
# SQL comments are forbidden in column aliases. # SQL comments are forbidden in column aliases.
FORBIDDEN_ALIAS_PATTERN = _lazy_re_compile(r"['`\"\]\[;\s]|--|/\*|\*/") FORBIDDEN_ALIAS_PATTERN = _lazy_re_compile(r"['`\"\]\[;\s]|--|/\*|\*/")
# Inspired from
# https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS
EXPLAIN_OPTIONS_PATTERN = _lazy_re_compile(r"[\w\-]+")
def get_field_names_from_opts(opts): def get_field_names_from_opts(opts):
if opts is None: if opts is None:
@ -589,6 +593,12 @@ class Query(BaseExpression):
def explain(self, using, format=None, **options): def explain(self, using, format=None, **options):
q = self.clone() q = self.clone()
for option_name in options:
if (
not EXPLAIN_OPTIONS_PATTERN.fullmatch(option_name)
or "--" in option_name
):
raise ValueError(f"Invalid option name: {option_name!r}.")
q.explain_info = ExplainInfo(format, options) q.explain_info = ExplainInfo(format, options)
compiler = q.get_compiler(using=using) compiler = q.get_compiler(using=using)
return "\n".join(compiler.explain_query()) return "\n".join(compiler.explain_query())

View File

@ -13,3 +13,10 @@ CVE-2022-28346: Potential SQL injection in ``QuerySet.annotate()``, ``aggregate(
:meth:`~.QuerySet.extra` methods were subject to SQL injection in column :meth:`~.QuerySet.extra` methods were subject to SQL injection in column
aliases, using a suitably crafted dictionary, with dictionary expansion, as the aliases, using a suitably crafted dictionary, with dictionary expansion, as the
``**kwargs`` passed to these methods. ``**kwargs`` passed to these methods.
CVE-2022-28347: Potential SQL injection via ``QuerySet.explain(**options)`` on PostgreSQL
=========================================================================================
:meth:`.QuerySet.explain` method was subject to SQL injection in option names,
using a suitably crafted dictionary, with dictionary expansion, as the
``**options`` argument.

View File

@ -15,6 +15,13 @@ CVE-2022-28346: Potential SQL injection in ``QuerySet.annotate()``, ``aggregate(
aliases, using a suitably crafted dictionary, with dictionary expansion, as the aliases, using a suitably crafted dictionary, with dictionary expansion, as the
``**kwargs`` passed to these methods. ``**kwargs`` passed to these methods.
CVE-2022-28347: Potential SQL injection via ``QuerySet.explain(**options)`` on PostgreSQL
=========================================================================================
:meth:`.QuerySet.explain` method was subject to SQL injection in option names,
using a suitably crafted dictionary, with dictionary expansion, as the
``**options`` argument.
Bugfixes Bugfixes
======== ========

View File

@ -15,6 +15,13 @@ CVE-2022-28346: Potential SQL injection in ``QuerySet.annotate()``, ``aggregate(
aliases, using a suitably crafted dictionary, with dictionary expansion, as the aliases, using a suitably crafted dictionary, with dictionary expansion, as the
``**kwargs`` passed to these methods. ``**kwargs`` passed to these methods.
CVE-2022-28347: Potential SQL injection via ``QuerySet.explain(**options)`` on PostgreSQL
=========================================================================================
:meth:`.QuerySet.explain` method was subject to SQL injection in option names,
using a suitably crafted dictionary, with dictionary expansion, as the
``**options`` argument.
Bugfixes Bugfixes
======== ========

View File

@ -59,8 +59,8 @@ class ExplainTests(TestCase):
@skipUnlessDBFeature("validates_explain_options") @skipUnlessDBFeature("validates_explain_options")
def test_unknown_options(self): def test_unknown_options(self):
with self.assertRaisesMessage(ValueError, "Unknown options: test, test2"): with self.assertRaisesMessage(ValueError, "Unknown options: TEST, TEST2"):
Tag.objects.explain(test=1, test2=1) Tag.objects.explain(**{"TEST": 1, "TEST2": 1})
def test_unknown_format(self): def test_unknown_format(self):
msg = "DOES NOT EXIST is not a recognized format." msg = "DOES NOT EXIST is not a recognized format."
@ -94,6 +94,35 @@ class ExplainTests(TestCase):
option = "{} {}".format(name.upper(), "true" if value else "false") option = "{} {}".format(name.upper(), "true" if value else "false")
self.assertIn(option, captured_queries[0]["sql"]) self.assertIn(option, captured_queries[0]["sql"])
def test_option_sql_injection(self):
qs = Tag.objects.filter(name="test")
options = {"SUMMARY true) SELECT 1; --": True}
msg = "Invalid option name: 'SUMMARY true) SELECT 1; --'"
with self.assertRaisesMessage(ValueError, msg):
qs.explain(**options)
def test_invalid_option_names(self):
qs = Tag.objects.filter(name="test")
tests = [
'opt"ion',
"o'ption",
"op`tion",
"opti on",
"option--",
"optio\tn",
"o\nption",
"option;",
"你 好",
# [] are used by MSSQL.
"option[",
"option]",
]
for invalid_option in tests:
with self.subTest(invalid_option):
msg = f"Invalid option name: {invalid_option!r}"
with self.assertRaisesMessage(ValueError, msg):
qs.explain(**{invalid_option: True})
@unittest.skipUnless(connection.vendor == "mysql", "MySQL specific") @unittest.skipUnless(connection.vendor == "mysql", "MySQL specific")
def test_mysql_text_to_traditional(self): def test_mysql_text_to_traditional(self):
# Ensure these cached properties are initialized to prevent queries for # Ensure these cached properties are initialized to prevent queries for