[Django] #29525: Handle strings as allowed_hosts arguments to is_safe_url()

[Django]

#29525: Handle strings as allowed_hosts arguments to is_safe_url()
------------------------------------------------+------------------------
Reporter: Przemysław Suliga | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Utilities | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
`is_safe_url()` expects a sequence of strings as its `allowed_hosts`
argument like this

{{{
>>> is_safe_url('http://example.com/abc', allowed_hosts={'example.com'})
True
}}}

When `allowed_hosts` is passed in incorrectly as a string instead of as a
sequence of strings like this

{{{
>>> is_safe_url('http://good.co/evil', allowed_hosts='good.com')
True
}}}

`is_safe_url()` will return `True` for some cases which might be
exploited.

Since `is_safe_url()` is not Django's public API, I decided to not go via
`secu...@djangoproject.com`.

Proposed solution is in https://github.com/django/django/pull/10082

--
Ticket URL: <https://code.djangoproject.com/ticket/29525>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#29525: Handle strings as allowed_hosts arguments to is_safe_url()

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

Reporter: Przemysław Suliga | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Utilities | Version: master

Severity: Normal | 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 Tim Graham):

* cc: Jon Dufresne (added)
* stage: Unreviewed => Accepted

Comment:

Jon, I'd like to hear your opinion on this. I suppose the function could
at least do type checking to prevent that mistake.

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

[Django]

#29525: Handle strings as allowed_hosts arguments to is_safe_url()

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

Reporter: Przemysław Suliga | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Utilities | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

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

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

Comment (by Jon Dufresne):

Type checking is fine by me and helps avoid a potentially unsafe mistake.
+1

Whether that type checking results in coercing (as done in the PR) or
raises an exception, I have no strong opinion. Either is sufficient to
prevent mistakes. I guess an exception is a bit more explicit, but, again,
no strong opinion.

--
Ticket URL: <https://code.djangoproject.com/ticket/29525#comment:2>

[Django]

#29525: Handle strings as allowed_hosts arguments to is_safe_url()

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

Reporter: Przemysław Suliga | Owner: nobody

Type: Cleanup/optimization | Status: closed
Component: Utilities | Version: master
Severity: Normal | Resolution: fixed
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 Tim Graham <timograham@…>):

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

Comment:

In [changeset:"d22b90b4eabc1fe9b7b35aada441e0edf5ebd6d8" d22b90b4]:
{{{
#!CommitTicketReference repository=""
revision="d22b90b4eabc1fe9b7b35aada441e0edf5ebd6d8"
Fixed #29525 -- Allowed is_safe_url()'s allowed_hosts arg to be a string.
}}}

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