[Django] #37095: URL redirect max length check should check %-encoded value

[Django]

#37095: URL redirect max length check should check %-encoded value
-------------------------------------------+-----------------------------
Reporter: Jacob Walls | Owner: Jacob Walls
Type: Bug | Status: assigned
Component: HTTP handling | Version: dev
Severity: Release blocker | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------------+-----------------------------
In #36767 we made configurable the length check on redirect URLs we added
for CVE-2025-64458. "Cowork" sent us a flurry of nuisance security reports
last night, but among them was a reasonable suggestion that we apply the
length check against the percent-encoded URI:

{{{#!diff
diff --git a/django/http/response.py b/django/http/response.py
index 45fb0177d1..0a61fb723f 100644
--- a/django/http/response.py
+++ b/django/http/response.py
@@ -641,12 +641,13 @@ class HttpResponseRedirectBase(HttpResponse):
**kwargs,
):
super().__init__(*args, **kwargs)
- self["Location"] = iri_to_uri(redirect_to)
redirect_to_str = str(redirect_to)
- if max_length is not None and len(redirect_to_str) > max_length:
+ location = iri_to_uri(redirect_to_str)
+ if max_length is not None and len(location) > max_length:
raise DisallowedRedirect(
f"Unsafe redirect exceeding {max_length} characters"
)
+ self["Location"] = location
parsed = urlsplit(redirect_to_str)
if preserve_request:
self.status_code = self.status_code_preserve_request
diff --git a/tests/httpwrappers/tests.py b/tests/httpwrappers/tests.py
index b990e9f816..3430dbbcd3 100644
--- a/tests/httpwrappers/tests.py
+++ b/tests/httpwrappers/tests.py
@@ -24,6 +24,7 @@ from django.http import (
parse_cookie,
)
from django.test import SimpleTestCase
+from django.utils.encoding import iri_to_uri
from django.utils.functional import lazystr
from django.utils.http import MAX_URL_REDIRECT_LENGTH

@@ -498,6 +499,29 @@ class HttpResponseTests(SimpleTestCase):
response = response_class(long_url)
self.assertEqual(response.url, long_url)

+ def test_redirect_url_max_length_checks_encoded_location(self):
+ long_url = "/" + "é" * (MAX_URL_REDIRECT_LENGTH - 1)
+ self.assertLessEqual(len(long_url), MAX_URL_REDIRECT_LENGTH)
+ self.assertGreater(len(iri_to_uri(long_url)),
MAX_URL_REDIRECT_LENGTH)
+ for response_class in (HttpResponseRedirect,
HttpResponsePermanentRedirect):
+ with (
+ self.subTest(response_class=response_class),
+ self.assertRaisesMessage(
+ DisallowedRedirect,
+ f"Unsafe redirect exceeding {MAX_URL_REDIRECT_LENGTH}
characters",
+ ),
+ ):
+ response_class(long_url)
+
+ def
test_redirect_url_max_length_allows_encoded_location_at_limit(self):
+ redirect_to = "/" + "é" * ((MAX_URL_REDIRECT_LENGTH - 1) // 6)
+ location = iri_to_uri(redirect_to)
+ self.assertLessEqual(len(location), MAX_URL_REDIRECT_LENGTH)
+ for response_class in (HttpResponseRedirect,
HttpResponsePermanentRedirect):
+ with self.subTest(response_class=response_class):
+ response = response_class(redirect_to)
+ self.assertEqual(response.url, location)
+
def test_redirect_url_max_length_override_via_param(self):
base_url = "https://example.com/"
for (max_length, length), response_class in itertools.product(
}}}

Although we didn't take this as a security issue (the limit we apply on
the raw value is good enough for DoS), this seems like a functionality bug
to fix before the release.
--
Ticket URL: <https://code.djangoproject.com/ticket/37095>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#37095: URL redirect max length check should check %-encoded value

---------------------------------+---------------------------------------

Reporter: Jacob Walls | Owner: Jacob Walls
Type: Bug | Status: assigned
Component: HTTP handling | Version: dev

Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

---------------------------------+---------------------------------------
Changes (by Jake Howard):

* cc: Jake Howard (added)
* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/37095#comment:1>

[Django]

#37095: URL redirect max length check should check %-encoded value
---------------------------------+---------------------------------------
Reporter: Jacob Walls | Owner: Jacob Walls
Type: Bug | Status: assigned
Component: HTTP handling | Version: dev
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+---------------------------------------

Changes (by Jacob Walls):

* has_patch: 0 => 1

Comment:

[https://github.com/django/django/pull/21276 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/37095#comment:2>

[Django]

#37095: URL redirect max length check should check %-encoded value

-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Jacob
| Walls
Type: Bug | Status: assigned
Component: HTTP handling | Version: dev
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Jake Howard):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/37095#comment:3>

[Django]

#37095: URL redirect max length check should check %-encoded value
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
| Walls

Type: Bug | Status: closed

Component: HTTP handling | Version: dev

Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls <jacobtylerwalls@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"335c6d0129400eda792f3bec5c71bb28af5e5d37" 335c6d01]:
{{{#!CommitTicketReference repository=""
revision="335c6d0129400eda792f3bec5c71bb28af5e5d37"
Fixed #37095 -- Checked maximum redirect lengths against percent-encoded
URLs.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37095#comment:4>