#22486 closed Bug (fixed)
urlresolvers.reverse() security fix (1.5.6) breaks when a view is a partial
Reported by: | Robert Coup | Owned by: | Tim Graham |
---|---|---|---|
Component: | Core (URLs) | Version: | dev |
Severity: | Release blocker | Keywords: | |
Cc: | luke@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The changes in the recent security fix for reverse()
Fixed a remote code execution vulnerabilty in URL reversing (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)
Change History (11)
comment:1 by , 11 years ago
comment:2 by , 11 years ago
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?
comment:3 by , 11 years ago
Has patch: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
Version: | 1.5 → 1.4 |
Looks good. We should probably backport this to all the branches where we applied the security fix.
comment:4 by , 11 years ago
Cc: | added |
---|
comment:5 by , 11 years ago
Severity: | Normal → Release blocker |
---|---|
Version: | 1.4 → master |
comment:6 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:7 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Are you suggesting a change in Django itself or would a documentation update that notes this restriction be sufficient?