Skip to content

Commit

Permalink
[3.1.x] Fixed CVE-2021-35042 -- Prevented SQL injection in QuerySet.o…
Browse files Browse the repository at this point in the history
…rder_by().

Regression introduced in 5139487
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 06eec31
per the 4.0 deprecation life cycle.

Thanks Joel Saunders for the report.
  • Loading branch information
charettes authored and felixxm committed Jul 1, 2021
1 parent 8dc1cc0 commit 0bd57a8
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 3 deletions.
2 changes: 2 additions & 0 deletions django/db/models/sql/constants.py
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -18,6 +19,7 @@
'ASC': ('ASC', 'DESC'),
'DESC': ('DESC', 'ASC'),
}
ORDER_PATTERN = _lazy_re_compile(r'[-+]?[.\w]+$')

# SQL join types.
INNER = 'INNER JOIN'
Expand Down
6 changes: 4 additions & 2 deletions django/db/models/sql/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
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,
)
Expand Down Expand Up @@ -1897,7 +1899,7 @@ def add_ordering(self, *ordering):
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 '
Expand Down
14 changes: 13 additions & 1 deletion docs/releases/3.1.13.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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.
8 changes: 8 additions & 0 deletions tests/queries/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -3134,6 +3134,14 @@ def test_invalid_order_by(self):
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):
Expand Down

0 comments on commit 0bd57a8

Please sign in to comment.