From 4dea4883e6c50d75f215a6b9bcbd95273f57c72d Mon Sep 17 00:00:00 2001 From: Florian Apolloner Date: Mon, 30 Jul 2012 22:03:46 +0200 Subject: [PATCH] [1.3.x] Fixed a security issue in http redirects. Disclosure and new release forthcoming. Backport of 4129201c3e0fa057c198bdefcb34686a23b4a93c from master. --- django/http/__init__.py | 25 ++++++++++++--------- tests/regressiontests/httpwrappers/tests.py | 19 ++++++++++++++-- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/django/http/__init__.py b/django/http/__init__.py index 07e5a46797..74113b080a 100644 --- a/django/http/__init__.py +++ b/django/http/__init__.py @@ -4,7 +4,7 @@ import re import time from pprint import pformat from urllib import urlencode, quote -from urlparse import urljoin +from urlparse import urljoin, urlparse try: from cStringIO import StringIO except ImportError: @@ -117,6 +117,7 @@ class CompatCookie(SimpleCookie): warnings.warn("CompatCookie is deprecated, use django.http.SimpleCookie instead.", PendingDeprecationWarning) +from django.core.exceptions import SuspiciousOperation from django.utils.datastructures import MultiValueDict, ImmutableList from django.utils.encoding import smart_str, iri_to_uri, force_unicode from django.utils.http import cookie_date @@ -635,20 +636,22 @@ class HttpResponse(object): raise Exception("This %s instance cannot tell its position" % self.__class__) return sum([len(chunk) for chunk in self._container]) -class HttpResponseRedirect(HttpResponse): +class HttpResponseRedirectBase(HttpResponse): + allowed_schemes = ['http', 'https', 'ftp'] + + def __init__(self, redirect_to): + super(HttpResponseRedirectBase, self).__init__() + parsed = urlparse(redirect_to) + if parsed.scheme and parsed.scheme not in self.allowed_schemes: + raise SuspiciousOperation("Unsafe redirect to URL with scheme '%s'" % parsed.scheme) + self['Location'] = iri_to_uri(redirect_to) + +class HttpResponseRedirect(HttpResponseRedirectBase): status_code = 302 - def __init__(self, redirect_to): - super(HttpResponseRedirect, self).__init__() - self['Location'] = iri_to_uri(redirect_to) - -class HttpResponsePermanentRedirect(HttpResponse): +class HttpResponsePermanentRedirect(HttpResponseRedirectBase): status_code = 301 - def __init__(self, redirect_to): - super(HttpResponsePermanentRedirect, self).__init__() - self['Location'] = iri_to_uri(redirect_to) - class HttpResponseNotModified(HttpResponse): status_code = 304 diff --git a/tests/regressiontests/httpwrappers/tests.py b/tests/regressiontests/httpwrappers/tests.py index 6aabfe655e..9a9b73edee 100644 --- a/tests/regressiontests/httpwrappers/tests.py +++ b/tests/regressiontests/httpwrappers/tests.py @@ -1,8 +1,11 @@ import copy import pickle -from django.http import (QueryDict, HttpResponse, SimpleCookie, BadHeaderError, - parse_cookie) +from django.core.exceptions import SuspiciousOperation +from django.http import (QueryDict, HttpResponse, HttpResponseRedirect, + HttpResponsePermanentRedirect, + SimpleCookie, BadHeaderError, + parse_cookie) from django.utils import unittest class QueryDictTests(unittest.TestCase): @@ -243,6 +246,18 @@ class HttpResponseTests(unittest.TestCase): self.assertRaises(BadHeaderError, r.__setitem__, 'test\rstr', 'test') self.assertRaises(BadHeaderError, r.__setitem__, 'test\nstr', 'test') + def test_unsafe_redirects(self): + bad_urls = [ + 'data:text/html,', + 'mailto:test@example.com', + 'file:///etc/passwd', + ] + for url in bad_urls: + self.assertRaises(SuspiciousOperation, + HttpResponseRedirect, url) + self.assertRaises(SuspiciousOperation, + HttpResponsePermanentRedirect, url) + class CookieTests(unittest.TestCase): def test_encode(self): """