Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#26902 closed New feature (fixed)

Add `secure` argument to `is_safe_url()`

Reported by: Przemysław Suliga Owned by: nobody
Component: Utilities Version: dev
Severity: Normal Keywords:
Cc: berker.peksag@… 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

django.utils.http.is_safe_url() considers any HTTP and HTTPS url safe as long as its hostname matches the host argument. Currently this is true: is_safe_url('http://example.com', host='example.com').

Let's add a secure argument to is_safe_url() so that when it's True, only HTTPS is considered as a safe scheme.

The existence of that argument alone would make users aware of potential issues that can arise from ignoring it. For example if a developer uses is_safe_url() to validate user supplied urls for redirection to a target with appended secrets as url query params.

django.contrib.admin uses django.contrib.auth login view where is_safe_url() is used to validate the next query param. This scenario is currently possible:

Of course our HTTPS site should only set Secure session cookies and use HSTS, so there should be no possibility of the the cookie being sent by the user via HTTP. But if the site doesn't set secure cookies and doesn't use HSTS, this is a problem. If the site doesn't use secure cookies in the first place, then the secure param to is_safe_url() won't help much.. but I would argue it still makes the validation more "complete".

Change History (5)

comment:1 by Tim Graham, 8 years ago

Triage Stage: UnreviewedAccepted
Version 0, edited 8 years ago by Tim Graham (next)

comment:2 by Berker Peksag, 8 years ago

Cc: berker.peksag@… added
Triage Stage: AcceptedReady for checkin

PR #6923 looks good to me. I just left two minor comments about the PR.

comment:3 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: newclosed

In 5e5a1702:

Fixed #26902 -- Allowed is_safe_url() to require an https URL.

Thanks Andrew Nester, Berker Peksag, and Tim Graham for reviews.

comment:4 by Tim Graham <timograham@…>, 8 years ago

In 549b90fa:

Refs #26902 -- Protected against insecure redirects in Login/LogoutView.

comment:5 by Tim Graham <timograham@…>, 8 years ago

In 1f68bb56:

Refs #26902 -- Protected against insecure redirects in set_language().

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