Author: jacob Date: 2010-03-01 13:58:53 -0600 (Mon, 01 Mar 2010) New Revision: 12635
Modified: django/trunk/django/contrib/auth/tests/views.py django/trunk/django/contrib/auth/views.py Log: Fixed #11457: tightened the security check for "next" redirects after logins. The new behavior still disallows redirects to off-site URLs, but now allows redirects of the form `/some/other/view?foo=http://...`. Thanks to brutasse. Modified: django/trunk/django/contrib/auth/tests/views.py =================================================================== --- django/trunk/django/contrib/auth/tests/views.py 2010-03-01 19:49:05 UTC (rev 12634) +++ django/trunk/django/contrib/auth/tests/views.py 2010-03-01 19:58:53 UTC (rev 12635) @@ -1,8 +1,9 @@ import os import re +import urllib from django.conf import settings -from django.contrib.auth import SESSION_KEY +from django.contrib.auth import SESSION_KEY, REDIRECT_FIELD_NAME from django.contrib.auth.forms import AuthenticationForm from django.contrib.sites.models import Site, RequestSite from django.contrib.auth.models import User @@ -183,6 +184,46 @@ self.assertEquals(response.context['site_name'], site.name) self.assert_(isinstance(response.context['form'], AuthenticationForm), 'Login form is not an AuthenticationForm') + + def test_security_check(self, password='password'): + login_url = reverse('django.contrib.auth.views.login') + + # Those URLs should not pass the security check + for bad_url in ('http://example.com', + 'https://example.com', + 'ftp://exampel.com', + '//example.com'): + + nasty_url = '%(url)s?%(next)s=%(bad_url)s' % { + 'url': login_url, + 'next': REDIRECT_FIELD_NAME, + 'bad_url': urllib.quote(bad_url) + } + response = self.client.post(nasty_url, { + 'username': 'testclient', + 'password': password, + } + ) + self.assertEquals(response.status_code, 302) + self.assertFalse(bad_url in response['Location'], "%s should be blocked" % bad_url) + + # Now, these URLs have an other URL as a GET parameter and therefore + # should be allowed + for url_ in ('http://example.com', 'https://example.com', + 'ftp://exampel.com', '//example.com'): + safe_url = '%(url)s?%(next)s=/view/?param=%(safe_param)s' % { + 'url': login_url, + 'next': REDIRECT_FIELD_NAME, + 'safe_param': urllib.quote(url_) + } + response = self.client.post(safe_url, { + 'username': 'testclient', + 'password': password, + } + ) + self.assertEquals(response.status_code, 302) + self.assertTrue('/view/?param=%s' % url_ in response['Location'], "/view/?param=%s should be allowed" % url_) + class LogoutTest(AuthViewsTestCase): urls = 'django.contrib.auth.tests.urls' Modified: django/trunk/django/contrib/auth/views.py =================================================================== --- django/trunk/django/contrib/auth/views.py 2010-03-01 19:49:05 UTC (rev 12634) +++ django/trunk/django/contrib/auth/views.py 2010-03-01 19:58:53 UTC (rev 12635) @@ -1,5 +1,8 @@ +import re from django.conf import settings from django.contrib.auth import REDIRECT_FIELD_NAME +# Avoid shadowing the login() view below. +from django.contrib.auth import login as auth_login from django.contrib.auth.decorators import login_required from django.contrib.auth.forms import AuthenticationForm from django.contrib.auth.forms import PasswordResetForm, SetPasswordForm, PasswordChangeForm @@ -20,26 +23,42 @@ def login(request, template_name='registration/login.html', redirect_field_name=REDIRECT_FIELD_NAME, authentication_form=AuthenticationForm): - "Displays the login form and handles the login action." + """Displays the login form and handles the login action.""" + redirect_to = request.REQUEST.get(redirect_field_name, '') + if request.method == "POST": form = authentication_form(data=request.POST) if form.is_valid(): # Light security check -- make sure redirect_to isn't garbage. - if not redirect_to or '//' in redirect_to or ' ' in redirect_to: + if not redirect_to or ' ' in redirect_to: redirect_to = settings.LOGIN_REDIRECT_URL - from django.contrib.auth import login - login(request, form.get_user()) + + # Heavier security check -- redirects to http://example.com should + # not be allowed, but things like /view/?param=http://example.com + # should be allowed. This regex checks if there is a '//' *before* a + # question mark. + elif '//' in redirect_to and re.match(r'[^\?]*//', redirect_to): + redirect_to = settings.LOGIN_REDIRECT_URL + + # Okay, security checks complete. Log the user in. + auth_login(request, form.get_user()) + if request.session.test_cookie_worked(): request.session.delete_test_cookie() + return HttpResponseRedirect(redirect_to) + else: form = authentication_form(request) + request.session.set_test_cookie() + if Site._meta.installed: current_site = Site.objects.get_current() else: current_site = RequestSite(request) + return render_to_response(template_name, { 'form': form, redirect_field_name: redirect_to, -- You received this message because you are subscribed to the Google Groups "Django updates" group. To post to this group, send email to django-upda...@googlegroups.com. To unsubscribe from this group, send email to django-updates+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-updates?hl=en.
