Opened 4 months ago
Closed 2 weeks ago
#36000 closed Cleanup/optimization (fixed)
Update default from http to https in urlize when protocol not provided
Description (last modified by ) ¶
In django/utils/html.py
, urlize
there is:
url = smart_urlquote("http://%s" % html.unescape(middle))
When user input does not include a protocol it defaults to http (Insecure Protocol).
Example :
Considered a web app using urlize()
for password reset email template
input = "Password reset link myapp.com/password/reset/{token}"
output:
"Password reset link <a href="http://myapp.com/password/reset/{token}"/>"
so when end user of myapp clicks it the url with token sent in http insecure protocol.
This behavior could potentially lead to man-in-the-middle attacks
Suggested Fix:
Default to HTTPS: If the URL doesn't specify a protocol, Django could default to https://
Change History (22)
comment:1 by , 4 months ago
Description: | modified (diff) |
---|---|
Has patch: | unset |
Summary: | Insecure URL Handling (HTTP Protocol Default) in urlize → Update default from http to https in urlize when protocol not provided |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 4 months ago
Component: | HTTP handling → Template system |
---|
comment:3 by , 4 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:4 by , 3 months ago
I think we can use a plan similar to how #34380 shook out, with a plan like:
- Introduce a transitional setting (
URLIZE_ASSUME_HTTPS
) that defaults toFalse
. This goes on the deprecation plan for removal in N+2 versions. - When the responsible code path is hit (which should be fairly rare as it only applies to limited domains), check the setting. If it’s
False
, warn and use 'http', otherwise use 'https'.
comment:8 by , 4 weeks ago
comment:9 by , 4 weeks ago
Owner: | changed from | to
---|
comment:10 by , 4 weeks ago
The issue regarding defaulting to HTTP instead of HTTPS in urlize has been addressed. A fix has been implemented to ensure that URLs without a specified protocol now default to HTTPS, improving security and preventing potential MITM attacks.
🔗 Pull Request: https://github.com/django/django/pull/19240
Summary of Fix:
- Added URLIZE_ASSUME_HTTPS setting (default: False) to control protocol selection
- Deprecated http:// default in Django 5.2
- Will make https:// the default in Django 6.0 (following deprecation policy)
- Added comprehensive tests and documentation
- Ensures backward compatibility during transition
- Improves security by preventing MITM attacks
Implementation follows Django's deprecation policy (N+2 versions) for a smooth transition. All tests are passing, and documentation has been updated accordingly.
Looking forward to your review and feedback!
comment:11 by , 4 weeks ago
Has patch: | set |
---|
comment:12 by , 3 weeks ago
Patch needs improvement: | set |
---|
comment:13 by , 3 weeks ago
I have completed all requested changes in the PR https://github.com/django/django/pull/19240, including:
Addressing all review comments.
Pushing a commit with the latest updates.
Ensuring all tests pass successfully.
Updating the documentation to reflect the introduced URLIZE_ASSUME_HTTPS
setting.
Additionally, I have added and updated some tests and documentation to ensure proper coverage and clarity. Given that the Trac ticket currently has "Needs tests: No" and "Needs documentation: No", should these be updated to "Needs tests: Yes" and "Needs documentation: Yes" to reflect these additions?
comment:14 by , 3 weeks ago
Additionally, I have added and updated some tests and documentation to ensure proper coverage and clarity. Given that the Trac ticket currently has "Needs tests: No" and "Needs documentation: No", should these be updated to "Needs tests: Yes" and "Needs documentation: Yes" to reflect these additions?
Yes when you want another review, you should do this as it will put the ticket in the review queue. I have given a review, so once you've resolved those comments you can update this ticket
comment:15 by , 3 weeks ago
Thank you for the review and clarification.
- Addressed and resolved all comments in the PR https://github.com/django/django/pull/19240
- Ensured passing all tests and documentation checks correctly.
Looking forward to your review and feedback!
comment:16 by , 3 weeks ago
Needs documentation: | set |
---|---|
Needs tests: | set |
comment:17 by , 3 weeks ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
comment:18 by , 2 weeks ago
Patch needs improvement: | unset |
---|
comment:19 by , 2 weeks ago
Patch needs improvement: | set |
---|
comment:20 by , 2 weeks ago
Patch needs improvement: | unset |
---|
comment:21 by , 2 weeks ago
Triage Stage: | Accepted → Ready for checkin |
---|
Thank you!
Note that the security team discussed this and agreed this can be handled publicly. This is similar to #34380.