diff --git a/django/db/backends/postgresql/features.py b/django/db/backends/postgresql/features.py index 5c8701c396..9f63ca6b0c 100644 --- a/django/db/backends/postgresql/features.py +++ b/django/db/backends/postgresql/features.py @@ -53,7 +53,6 @@ class DatabaseFeatures(BaseDatabaseFeatures): supports_over_clause = True supports_aggregate_filter_clause = True supported_explain_formats = {'JSON', 'TEXT', 'XML', 'YAML'} - validates_explain_options = False # A query will error on invalid options. @cached_property def is_postgresql_9_5(self): diff --git a/django/db/backends/postgresql/operations.py b/django/db/backends/postgresql/operations.py index 66e5482be6..66ac2d5d10 100644 --- a/django/db/backends/postgresql/operations.py +++ b/django/db/backends/postgresql/operations.py @@ -8,6 +8,18 @@ from django.db.backends.base.operations import BaseDatabaseOperations class DatabaseOperations(BaseDatabaseOperations): cast_char_field_without_max_length = 'varchar' explain_prefix = 'EXPLAIN' + explain_options = frozenset( + [ + "ANALYZE", + "BUFFERS", + "COSTS", + "SETTINGS", + "SUMMARY", + "TIMING", + "VERBOSE", + "WAL", + ] + ) cast_data_types = { 'AutoField': 'integer', 'BigAutoField': 'bigint', @@ -267,15 +279,20 @@ class DatabaseOperations(BaseDatabaseOperations): return start_, end_ def explain_query_prefix(self, format=None, **options): - prefix = super().explain_query_prefix(format) extra = {} - if format: - extra['FORMAT'] = format + # Normalize options. if options: - extra.update({ + 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: + extra['FORMAT'] = format if extra: prefix += ' (%s)' % ', '.join('%s %s' % i for i in extra.items()) return prefix diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 412e817f10..1e823cfe74 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -45,6 +45,10 @@ __all__ = ['Query', 'RawQuery'] # SQL comments are forbidden in column aliases. FORBIDDEN_ALIAS_PATTERN = re.compile(r"['`\"\]\[;\s]|--|/\*|\*/") +# Inspired from +# https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS +EXPLAIN_OPTIONS_PATTERN = re.compile(r"[\w\-]+") + def get_field_names_from_opts(opts): return set(chain.from_iterable( @@ -528,6 +532,12 @@ class Query: def explain(self, using, format=None, **options): q = self.clone() + for option_name in options: + if ( + not EXPLAIN_OPTIONS_PATTERN.fullmatch(option_name) or + "--" in option_name + ): + raise ValueError("Invalid option name: '%s'." % option_name) q.explain_query = True q.explain_format = format q.explain_options = options diff --git a/docs/releases/2.2.28.txt b/docs/releases/2.2.28.txt index a894bddb3c..43270fc5c0 100644 --- a/docs/releases/2.2.28.txt +++ b/docs/releases/2.2.28.txt @@ -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 aliases, using a suitably crafted dictionary, with dictionary expansion, as the ``**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. diff --git a/tests/queries/test_explain.py b/tests/queries/test_explain.py index 9428bd88e9..209c192307 100644 --- a/tests/queries/test_explain.py +++ b/tests/queries/test_explain.py @@ -41,8 +41,8 @@ class ExplainTests(TestCase): @skipUnlessDBFeature('validates_explain_options') def test_unknown_options(self): - with self.assertRaisesMessage(ValueError, 'Unknown options: test, test2'): - Tag.objects.all().explain(test=1, test2=1) + with self.assertRaisesMessage(ValueError, "Unknown options: TEST, TEST2"): + Tag.objects.all().explain(**{"TEST": 1, "TEST2": 1}) def test_unknown_format(self): msg = 'DOES NOT EXIST is not a recognized format.' @@ -71,6 +71,35 @@ class ExplainTests(TestCase): option = '{} {}'.format(name.upper(), 'true' if value else 'false') 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 = "Invalid option name: '%s'" % invalid_option + with self.assertRaisesMessage(ValueError, msg): + qs.explain(**{invalid_option: True}) + @unittest.skipUnless(connection.vendor == 'mysql', 'MySQL specific') def test_mysql_text_to_traditional(self): # Initialize the cached property, if needed, to prevent a query for