Opened 3 days ago

Closed 3 days ago

#37095 closed Bug (fixed)

URL redirect max length check should check %-encoded value

Reported by: Jacob Walls Owned by: Jacob Walls
Component: HTTP handling Version: dev
Severity: Release blocker Keywords:
Cc: Varun Kasyap Pentamaraju, Jake Howard Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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:

  • django/http/response.py

    diff --git a/django/http/response.py b/django/http/response.py
    index 45fb0177d1..0a61fb723f 100644
    a b class HttpResponseRedirectBase(HttpResponse):  
    641641        **kwargs,
    642642    ):
    643643        super().__init__(*args, **kwargs)
    644         self["Location"] = iri_to_uri(redirect_to)
    645644        redirect_to_str = str(redirect_to)
    646         if max_length is not None and len(redirect_to_str) > max_length:
     645        location = iri_to_uri(redirect_to_str)
     646        if max_length is not None and len(location) > max_length:
    647647            raise DisallowedRedirect(
    648648                f"Unsafe redirect exceeding {max_length} characters"
    649649            )
     650        self["Location"] = location
    650651        parsed = urlsplit(redirect_to_str)
    651652        if preserve_request:
    652653            self.status_code = self.status_code_preserve_request
  • tests/httpwrappers/tests.py

    diff --git a/tests/httpwrappers/tests.py b/tests/httpwrappers/tests.py
    index b990e9f816..3430dbbcd3 100644
    a b from django.http import (  
    2424    parse_cookie,
    2525)
    2626from django.test import SimpleTestCase
     27from django.utils.encoding import iri_to_uri
    2728from django.utils.functional import lazystr
    2829from django.utils.http import MAX_URL_REDIRECT_LENGTH
    2930
    class HttpResponseTests(SimpleTestCase):  
    498499                response = response_class(long_url)
    499500                self.assertEqual(response.url, long_url)
    500501
     502    def test_redirect_url_max_length_checks_encoded_location(self):
     503        long_url = "/" + "é" * (MAX_URL_REDIRECT_LENGTH - 1)
     504        self.assertLessEqual(len(long_url), MAX_URL_REDIRECT_LENGTH)
     505        self.assertGreater(len(iri_to_uri(long_url)), MAX_URL_REDIRECT_LENGTH)
     506        for response_class in (HttpResponseRedirect, HttpResponsePermanentRedirect):
     507            with (
     508                self.subTest(response_class=response_class),
     509                self.assertRaisesMessage(
     510                    DisallowedRedirect,
     511                    f"Unsafe redirect exceeding {MAX_URL_REDIRECT_LENGTH} characters",
     512                ),
     513            ):
     514                response_class(long_url)
     515
     516    def test_redirect_url_max_length_allows_encoded_location_at_limit(self):
     517        redirect_to = "/" + "é" * ((MAX_URL_REDIRECT_LENGTH - 1) // 6)
     518        location = iri_to_uri(redirect_to)
     519        self.assertLessEqual(len(location), MAX_URL_REDIRECT_LENGTH)
     520        for response_class in (HttpResponseRedirect, HttpResponsePermanentRedirect):
     521            with self.subTest(response_class=response_class):
     522                response = response_class(redirect_to)
     523                self.assertEqual(response.url, location)
     524
    501525    def test_redirect_url_max_length_override_via_param(self):
    502526        base_url = "https://example.com/"
    503527        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.

Change History (4)

comment:1 by Jake Howard, 3 days ago

Cc: Jake Howard added
Triage Stage: UnreviewedAccepted

comment:2 by Jacob Walls, 3 days ago

Has patch: set

comment:3 by Jake Howard, 3 days ago

Triage Stage: AcceptedReady for checkin

comment:4 by Jacob Walls <jacobtylerwalls@…>, 3 days ago

Resolution: fixed
Status: assignedclosed

In 335c6d01:

Fixed #37095 -- Checked maximum redirect lengths against percent-encoded URLs.

Note: See TracTickets for help on using tickets.
Back to Top