summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon Charette <charette.s@gmail.com>2021-06-18 01:16:10 -0400
committerMariusz Felisiak <felisiak.mariusz@gmail.com>2021-07-01 08:36:17 +0200
commit0bd57a879a0d54920bb9038a732645fb917040e9 (patch)
tree6c2cf9581885bc4a39329adf3a78b17bf80abed6
parent8dc1cc0b306168eb1c0a0fc5457b6f1156fcbcff (diff)
downloaddjango-0bd57a879a0d54920bb9038a732645fb917040e9.tar.gz
[3.1.x] Fixed CVE-2021-35042 -- Prevented SQL injection in QuerySet.order_by().
Regression introduced in 513948735b799239f3ef8c89397592445e1a0cd5 by marking the raw SQL column reference feature for deprecation in Django 4.0 while lifting the column format validation. In retrospective the validation should have been kept around and the user should have been pointed at using RawSQL expressions during the deprecation period. The main branch is not affected because the raw SQL column reference support has been removed in 06eec3197009b88e3a633128bbcbd76eea0b46ff per the 4.0 deprecation life cycle. Thanks Joel Saunders for the report.
-rw-r--r--django/db/models/sql/constants.py2
-rw-r--r--django/db/models/sql/query.py6
-rw-r--r--docs/releases/3.1.13.txt14
-rw-r--r--tests/queries/tests.py8
4 files changed, 27 insertions, 3 deletions
diff --git a/django/db/models/sql/constants.py b/django/db/models/sql/constants.py
index a1db61b9ff..97edf7525e 100644
--- a/django/db/models/sql/constants.py
+++ b/django/db/models/sql/constants.py
@@ -1,6 +1,7 @@
"""
Constants specific to the SQL storage portion of the ORM.
"""
+from django.utils.regex_helper import _lazy_re_compile
# Size of each "chunk" for get_iterator calls.
# Larger values are slightly faster at the expense of more storage space.
@@ -18,6 +19,7 @@ ORDER_DIR = {
'ASC': ('ASC', 'DESC'),
'DESC': ('DESC', 'ASC'),
}
+ORDER_PATTERN = _lazy_re_compile(r'[-+]?[.\w]+$')
# SQL join types.
INNER = 'INNER JOIN'
diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index 7a16d4889a..2b5f1d8b85 100644
--- a/django/db/models/sql/query.py
+++ b/django/db/models/sql/query.py
@@ -30,7 +30,9 @@ from django.db.models.lookups import Lookup
from django.db.models.query_utils import (
Q, check_rel_lookup_compatibility, refs_expression,
)
-from django.db.models.sql.constants import INNER, LOUTER, ORDER_DIR, SINGLE
+from django.db.models.sql.constants import (
+ INNER, LOUTER, ORDER_DIR, ORDER_PATTERN, SINGLE,
+)
from django.db.models.sql.datastructures import (
BaseTable, Empty, Join, MultiJoin,
)
@@ -1897,7 +1899,7 @@ class Query(BaseExpression):
errors = []
for item in ordering:
if isinstance(item, str):
- if '.' in item:
+ if '.' in item and ORDER_PATTERN.match(item):
warnings.warn(
'Passing column raw column aliases to order_by() is '
'deprecated. Wrap %r in a RawSQL expression before '
diff --git a/docs/releases/3.1.13.txt b/docs/releases/3.1.13.txt
index 9f5b2b38cd..af8ccec535 100644
--- a/docs/releases/3.1.13.txt
+++ b/docs/releases/3.1.13.txt
@@ -6,4 +6,16 @@ Django 3.1.13 release notes
Django 3.1.13 fixes a security issues with severity "high" in 3.1.12.
-...
+CVE-2021-35042: Potential SQL injection via unsanitized ``QuerySet.order_by()`` input
+=====================================================================================
+
+Unsanitized user input passed to ``QuerySet.order_by()`` could bypass intended
+column reference validation in path marked for deprecation resulting in a
+potential SQL injection even if a deprecation warning is emitted.
+
+As a mitigation the strict column reference validation was restored for the
+duration of the deprecation period. This regression appeared in 3.1 as a side
+effect of fixing :ticket:`31426`.
+
+The issue is not present in the main branch as the deprecated path has been
+removed.
diff --git a/tests/queries/tests.py b/tests/queries/tests.py
index 11d5ec11ab..998072cd66 100644
--- a/tests/queries/tests.py
+++ b/tests/queries/tests.py
@@ -3134,6 +3134,14 @@ class QuerySetExceptionTests(SimpleTestCase):
with self.assertRaisesMessage(FieldError, msg):
Article.objects.order_by('*')
+ def test_order_by_escape_prevention(self):
+ msg = (
+ "Cannot resolve keyword 'queries.name);' into field. Choices are: "
+ "created, id, name"
+ )
+ with self.assertRaisesMessage(FieldError, msg):
+ Article.objects.order_by('queries.name);')
+
def test_invalid_queryset_model(self):
msg = 'Cannot use QuerySet for "Article": Use a QuerySet for "ExtraInfo".'
with self.assertRaisesMessage(ValueError, msg):