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):