summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMariusz Felisiak <felisiak.mariusz@gmail.com>2022-04-01 13:48:47 +0200
committerMariusz Felisiak <felisiak.mariusz@gmail.com>2022-04-11 09:02:58 +0200
commit00b0fc50e1738c7174c495464a5ef069408a4402 (patch)
treecd5668d0ea1c96bbd62c5e922b0d1988c2287e90
parent800828887a0509ad1162d6d407e94d8de7eafc60 (diff)
downloaddjango-00b0fc50e1738c7174c495464a5ef069408a4402.tar.gz
[4.0.x] Fixed CVE-2022-28347 -- Protected QuerySet.explain(**options) against SQL injection on PostgreSQL.
Backport of 6723a26e59b0b5429a0c5873941e01a2e1bdbb81 from main.
-rw-r--r--django/db/backends/postgresql/features.py1
-rw-r--r--django/db/backends/postgresql/operations.py31
-rw-r--r--django/db/models/sql/query.py10
-rw-r--r--docs/releases/2.2.28.txt7
-rw-r--r--docs/releases/3.2.13.txt7
-rw-r--r--docs/releases/4.0.4.txt7
-rw-r--r--tests/queries/test_explain.py33
7 files changed, 85 insertions, 11 deletions
diff --git a/django/db/backends/postgresql/features.py b/django/db/backends/postgresql/features.py
index 61715f30d7..6dab8d73d5 100644
--- a/django/db/backends/postgresql/features.py
+++ b/django/db/backends/postgresql/features.py
@@ -54,7 +54,6 @@ class DatabaseFeatures(BaseDatabaseFeatures):
only_supports_unbounded_with_preceding_and_following = True
supports_aggregate_filter_clause = True
supported_explain_formats = {"JSON", "TEXT", "XML", "YAML"}
- validates_explain_options = False # A query will error on invalid options.
supports_deferrable_unique_constraints = True
has_json_operators = True
json_key_contains_list_matching_requires_list = True
diff --git a/django/db/backends/postgresql/operations.py b/django/db/backends/postgresql/operations.py
index 1cb6050bc5..f2d451026a 100644
--- a/django/db/backends/postgresql/operations.py
+++ b/django/db/backends/postgresql/operations.py
@@ -8,6 +8,18 @@ from django.db.backends.utils import split_tzname_delta
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",
@@ -288,17 +300,20 @@ class DatabaseOperations(BaseDatabaseOperations):
return super().subtract_temporals(internal_type, lhs, rhs)
def explain_query_prefix(self, format=None, **options):
- prefix = super().explain_query_prefix(format)
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:
extra["FORMAT"] = format
- if options:
- extra.update(
- {
- name.upper(): "true" if value else "false"
- for name, value in options.items()
- }
- )
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 46b4280695..fd4bbdaf35 100644
--- a/django/db/models/sql/query.py
+++ b/django/db/models/sql/query.py
@@ -49,6 +49,10 @@ __all__ = ["Query", "RawQuery"]
# SQL comments are forbidden in column aliases.
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):
return set(
@@ -586,6 +590,12 @@ class Query(BaseExpression):
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(f"Invalid option name: {option_name!r}.")
q.explain_info = ExplainInfo(format, options)
compiler = q.get_compiler(using=using)
return "\n".join(compiler.explain_query())
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/docs/releases/3.2.13.txt b/docs/releases/3.2.13.txt
index ee20aa2ca1..b7afbb8ed7 100644
--- a/docs/releases/3.2.13.txt
+++ b/docs/releases/3.2.13.txt
@@ -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
``**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
========
diff --git a/docs/releases/4.0.4.txt b/docs/releases/4.0.4.txt
index 6c22788bd1..702cebbbe9 100644
--- a/docs/releases/4.0.4.txt
+++ b/docs/releases/4.0.4.txt
@@ -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
``**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
========
diff --git a/tests/queries/test_explain.py b/tests/queries/test_explain.py
index 2acd6a685c..adbdfa5c39 100644
--- a/tests/queries/test_explain.py
+++ b/tests/queries/test_explain.py
@@ -59,8 +59,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."
@@ -94,6 +94,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 = f"Invalid option name: {invalid_option!r}"
+ with self.assertRaisesMessage(ValueError, msg):
+ qs.explain(**{invalid_option: True})
+
@unittest.skipUnless(connection.vendor == "mysql", "MySQL specific")
def test_mysql_text_to_traditional(self):
# Ensure these cached properties are initialized to prevent queries for