[Django] #22486: urlresolvers.reverse() security fix (1.5.6) breaks when a view is a partial
Django
-----------------------------+--------------------
Reporter: rcoup | Owner: nobody
Type: Bug | Status: new
Component: Core (URLs) | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------
The changes in the recent security fix for `reverse()`
[https://github.com/django/django/commit/8b93b31487d6d3b0fcbbd0498991ea0db9088054
Fixed a remote code execution vulnerabilty in URL reversing]
([https://www.djangoproject.com/weblog/2014/apr/21/security/
CVE-2014-0472]) break when any view in the urlpatterns is a functools
partial.
Django 1.5.6 / Py2.6.5 / Linux.
pseudo-code:
{{{
# myapp/views.py
def my_view(request, **kwargs):
return HttpResponse(str(kwargs))
my_view2 = functools.partial(my_view, some_arg=123)
# myapp/urls.py
urlpatterns = patterns('my_app.views',
('^some_view/$', 'normal_view'), # a normal function/class view
('^breaks/$', 'my_view2'), # partial-wrapped view
)
}}}
Then
{{{
>>> from django.core.urlresolvers import reverse
>>> reverse('my_app.some_view')
django/core/urlresolvers.pyc in reverse(viewname, urlconf, args, kwargs,
prefix, current_app)
514 resolver = get_ns_resolver(ns_pattern, resolver)
515
--> 516 return iri_to_uri(resolver._reverse_with_prefix(view, prefix,
*args, **kwargs))
517
518 reverse_lazy = lazy(reverse, str)
django/core/urlresolvers.pyc in _reverse_with_prefix(self, lookup_view,
_prefix, *args, **kwargs)
393
394 if not self._populated:
--> 395 self._populate()
396
397 try:
django/core/urlresolvers.pyc in _populate(self)
285 else:
286 parent = normalize(pattern.regex.pattern)
--> 287 for name in pattern.reverse_dict:
288 for matches, pat, defaults in
pattern.reverse_dict.getlist(name):
289 new_matches = []
django/core/urlresolvers.pyc in reverse_dict(self)
310 language_code = get_language()
311 if language_code not in self._reverse_dict:
--> 312 self._populate()
313 return self._reverse_dict[language_code]
314
django/core/urlresolvers.pyc in _populate(self)
271 callback = pattern._callback
272 if not hasattr(callback, '__name__'):
--> 273 lookup_str = callback.__module__ + "." +
callback.__class__.__name__
274 else:
275 lookup_str = callback.__module__ + "." +
callback.__name__
AttributeError: 'functools.partial' object has no attribute '__module__'
}}}
Changing the view to use `functools.update_wrapper()` (as it probably
should) and it works okay:
{{{
my_view2 = functools.partial(my_view, some_arg=123)
functools.update_wrapper(my_view2, my_view)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22486>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: rcoup | Owner: nobody
Type: Bug | Status: new
Component: Core (URLs) | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Are you suggesting a change in Django itself or would a documentation
update that notes this restriction be sufficient?
--
Ticket URL: <https://code.djangoproject.com/ticket/22486#comment:1>
Django
Reporter: rcoup | Owner: nobody
Type: Bug | Status: new
Component: Core (URLs) | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by prestontimmons):
Although partials aren't reversible by the "path.to.view" syntax, I don't
think Django should blow up on them. Some parts of our code base use
partials extensively.
A simple fix might be to create `lookup_str` from the original function
whenever a pattern callback is a partial. The behavior would end up the
same as if update_wrapper were called.
A possible patch is here: https://github.com/django/django/pull/2601/
What do you think?
--
Ticket URL: <https://code.djangoproject.com/ticket/22486#comment:2>
Django
Reporter: rcoup | Owner: nobody
Type: Bug | Status: new
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 timo):
* has_patch: 0 => 1
* version: 1.5 => 1.4
* stage: Unreviewed => Accepted
Comment:
Looks good. We should probably backport this to all the branches where we
applied the security fix.
--
Ticket URL: <https://code.djangoproject.com/ticket/22486#comment:3>
Django
Reporter: rcoup | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by lfaraone):
* cc: luke@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/22486#comment:4>
Django
Reporter: rcoup | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by anonymous):
* version: 1.4 => master
* severity: Normal => Release blocker
--
Ticket URL: <https://code.djangoproject.com/ticket/22486#comment:5>
Django
Reporter: rcoup | Owner: timo
Type: Bug | Status: assigned
Component: Core (URLs) | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* owner: nobody => timo
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/22486#comment:6>
Django
Reporter: rcoup | Owner: timo
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"3c06b2f2a341922db4294b91117c7b1c34119a8c"]:
{{{
#!CommitTicketReference repository=""
revision="3c06b2f2a341922db4294b91117c7b1c34119a8c"
Fixed #22486 -- Restored the ability to reverse views created using
functools.partial.
Regression in 8b93b31487d6d3b0fcbbd0498991ea0db9088054.
Thanks rcoup for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22486#comment:7>
Django
Reporter: rcoup | Owner: timo
Type: Bug | Status: closed
Component: Core (URLs) | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"e192f131033d68e1b018263aee6fe8804a923b22"]:
{{{
#!CommitTicketReference repository=""
revision="e192f131033d68e1b018263aee6fe8804a923b22"
[1.7.x] Fixed #22486 -- Restored the ability to reverse views created
using functools.partial.
Regression in 8b93b31487d6d3b0fcbbd0498991ea0db9088054.
Thanks rcoup for the report.
Backport of 3c06b2f2a3 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22486#comment:8>
Django
Reporter: rcoup | Owner: timo
Type: Bug | Status: closed
Component: Core (URLs) | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"6915220ff9d6eeb2a669421d06bce9403ed6480c"]:
{{{
#!CommitTicketReference repository=""
revision="6915220ff9d6eeb2a669421d06bce9403ed6480c"
[1.6.x] Fixed #22486 -- Restored the ability to reverse views created
using functools.partial.
Regression in 8b93b31487d6d3b0fcbbd0498991ea0db9088054.
Thanks rcoup for the report.
Backport of 3c06b2f2a3 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22486#comment:9>
Django
Reporter: rcoup | Owner: timo
Type: Bug | Status: closed
Component: Core (URLs) | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"19bd6b9477e8f09f867640f72f3eb335cebe1d6a"]:
{{{
#!CommitTicketReference repository=""
revision="19bd6b9477e8f09f867640f72f3eb335cebe1d6a"
[1.5.x] Fixed #22486 -- Restored the ability to reverse views created
using functools.partial.
Regression in 8b93b31487d6d3b0fcbbd0498991ea0db9088054.
Thanks rcoup for the report.
Backport of 3c06b2f2a3 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22486#comment:10>
Django
Reporter: rcoup | Owner: timo
Type: Bug | Status: closed
Component: Core (URLs) | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"b91c385e324f1cb94d20e2ad146372c259d51d3b"]:
{{{
#!CommitTicketReference repository=""
revision="b91c385e324f1cb94d20e2ad146372c259d51d3b"
[1.4.x] Fixed #22486 -- Restored the ability to reverse views created
using functools.partial.
Regression in 8b93b31.
Thanks rcoup for the report.
Backport of 3c06b2f2a3 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22486#comment:11>