Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

[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 authored
43  django/contrib/auth/tests/views.py
... ...
@@ -1,8 +1,9 @@
1 1
 import os
2 2
 import re
  3
+import urllib
3 4
 
4 5
 from django.conf import settings
5  
-from django.contrib.auth import SESSION_KEY
  6
+from django.contrib.auth import SESSION_KEY, REDIRECT_FIELD_NAME
6 7
 from django.contrib.auth.forms import AuthenticationForm
7 8
 from django.contrib.sites.models import Site, RequestSite
8 9
 from django.contrib.auth.models import User
@@ -183,6 +184,46 @@ def test_current_site_in_context_after_login(self):
183 184
         self.assertEquals(response.context['site_name'], site.name)
184 185
         self.assert_(isinstance(response.context['form'], AuthenticationForm), 
185 186
                      'Login form is not an AuthenticationForm')
  187
+
  188
+    def test_security_check(self, password='password'):
  189
+        login_url = reverse('django.contrib.auth.views.login')
  190
+
  191
+        # Those URLs should not pass the security check
  192
+        for bad_url in ('http://example.com',
  193
+                        'https://example.com',
  194
+                        'ftp://exampel.com',
  195
+                        '//example.com'):
  196
+
  197
+            nasty_url = '%(url)s?%(next)s=%(bad_url)s' % {
  198
+                'url': login_url,
  199
+                'next': REDIRECT_FIELD_NAME,
  200
+                'bad_url': urllib.quote(bad_url)
  201
+            }
  202
+            response = self.client.post(nasty_url, {
  203
+                'username': 'testclient',
  204
+                'password': password,
  205
+                }
  206
+            )
  207
+            self.assertEquals(response.status_code, 302)
  208
+            self.assertFalse(bad_url in response['Location'], "%s should be blocked" % bad_url)
  209
+
  210
+        # Now, these URLs have an other URL as a GET parameter and therefore
  211
+        # should be allowed
  212
+        for url_ in ('http://example.com', 'https://example.com',
  213
+                    'ftp://exampel.com',  '//example.com'):
  214
+            safe_url = '%(url)s?%(next)s=/view/?param=%(safe_param)s' % {
  215
+                'url': login_url,
  216
+                'next': REDIRECT_FIELD_NAME,
  217
+                'safe_param': urllib.quote(url_)
  218
+            }
  219
+            response = self.client.post(safe_url, {
  220
+                    'username': 'testclient',
  221
+                    'password': password,
  222
+                }
  223
+            )
  224
+            self.assertEquals(response.status_code, 302)
  225
+            self.assertTrue('/view/?param=%s' % url_ in response['Location'], "/view/?param=%s should be allowed" % url_)
  226
+
186 227
         
187 228
 class LogoutTest(AuthViewsTestCase):
188 229
     urls = 'django.contrib.auth.tests.urls'
23  django/contrib/auth/views.py
... ...
@@ -1,5 +1,8 @@
  1
+import re
1 2
 from django.conf import settings
2 3
 from django.contrib.auth import REDIRECT_FIELD_NAME
  4
+# Avoid shadowing the login() view below.
  5
+from django.contrib.auth import login as auth_login
3 6
 from django.contrib.auth.decorators import login_required
4 7
 from django.contrib.auth.forms import AuthenticationForm
5 8
 from django.contrib.auth.forms import PasswordResetForm, SetPasswordForm, PasswordChangeForm
@@ -17,24 +20,38 @@
17 20
 def login(request, template_name='registration/login.html', redirect_field_name=REDIRECT_FIELD_NAME):
18 21
     "Displays the login form and handles the login action."
19 22
     redirect_to = request.REQUEST.get(redirect_field_name, '')
  23
+    
20 24
     if request.method == "POST":
21 25
         form = AuthenticationForm(data=request.POST)
22 26
         if form.is_valid():
23 27
             # Light security check -- make sure redirect_to isn't garbage.
24  
-            if not redirect_to or '//' in redirect_to or ' ' in redirect_to:
  28
+            if not redirect_to or ' ' in redirect_to:
25 29
                 redirect_to = settings.LOGIN_REDIRECT_URL
26  
-            from django.contrib.auth import login
27  
-            login(request, form.get_user())
  30
+            
  31
+            # Heavier security check -- redirects to http://example.com should 
  32
+            # not be allowed, but things like /view/?param=http://example.com 
  33
+            # should be allowed. This regex checks if there is a '//' *before* a
  34
+            # question mark.
  35
+            elif '//' in redirect_to and re.match(r'[^\?]*//', redirect_to):
  36
+                    redirect_to = settings.LOGIN_REDIRECT_URL
  37
+            
  38
+            # Okay, security checks complete. Log the user in.
  39
+            auth_login(request, form.get_user())
  40
+
28 41
             if request.session.test_cookie_worked():
29 42
                 request.session.delete_test_cookie()
  43
+
30 44
             return HttpResponseRedirect(redirect_to)
  45
+
31 46
     else:
32 47
         form = AuthenticationForm(request)
33 48
     request.session.set_test_cookie()
  49
+    
34 50
     if Site._meta.installed:
35 51
         current_site = Site.objects.get_current()
36 52
     else:
37 53
         current_site = RequestSite(request)
  54
+    
38 55
     return render_to_response(template_name, {
39 56
         'form': form,
40 57
         redirect_field_name: redirect_to,

0 notes on commit 6b94c64

Please sign in to comment.
Something went wrong with that request. Please try again.