Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

[1.1.X] Fixed #11457: tightened the security check for "next" redirec…

…ts 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.

Backport of [12635] from trunk.

git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.1.X@12636 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 6b94c64554b672cdc0ef4b812ce1c7636ce28210 1 parent 5f3a530
Jacob Kaplan-Moss jacobian authored
Showing with 62 additions and 4 deletions.
  1. +42 −1 django/contrib/auth/tests/views.py
  2. +20 −3 django/contrib/auth/views.py
43 django/contrib/auth/tests/views.py
View
@@ -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 @@ def test_current_site_in_context_after_login(self):
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'
23 django/contrib/auth/views.py
View
@@ -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
@@ -17,24 +20,38 @@
def login(request, template_name='registration/login.html', redirect_field_name=REDIRECT_FIELD_NAME):
"Displays the login form and handles the login action."
redirect_to = request.REQUEST.get(redirect_field_name, '')
+
if request.method == "POST":
form = AuthenticationForm(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 = AuthenticationForm(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,
Please sign in to comment.
Something went wrong with that request. Please try again.