#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 , 12 years ago
comment:2 by , 12 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 , 12 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 , 12 years ago
| Cc: | added |
|---|
comment:5 by , 12 years ago
| Severity: | Normal → Release blocker |
|---|---|
| Version: | 1.4 → master |
comment:6 by , 12 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:7 by , 12 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?