[1.5.X] Fixed #18856 -- Ensured that redirects can't be poisoned by malicious users.

6 files changed, 58 insertions, 50 deletions

diff --git a/django/contrib/auth/views.py b/django/contrib/auth/views.py
index 2562a639b7..8514345d00 100644
--- a/django/contrib/auth/views.py
+++ b/django/contrib/auth/views.py
@@ -7,7 +7,7 @@ from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.http import HttpResponseRedirect, QueryDict
 from django.template.response import TemplateResponse
-from django.utils.http import base36_to_int
+from django.utils.http import base36_to_int, is_safe_url
 from django.utils.translation import ugettext as _
 from django.shortcuts import resolve_url
 from django.views.decorators.debug import sensitive_post_parameters
@@ -37,18 +37,12 @@ def login(request, template_name='registration/login.html',
     if request.method == "POST":
         form = authentication_form(data=request.POST)
         if form.is_valid():
-            # Use default setting if redirect_to is empty
-            if not redirect_to:
-                redirect_to = settings.LOGIN_REDIRECT_URL
-            redirect_to = resolve_url(redirect_to)
 
-            netloc = urlparse(redirect_to)[1]
-            # Heavier security check -- don't allow redirection to a different
-            # host.
-            if netloc and netloc != request.get_host():
+            # Ensure the user-originating redirection url is safe.
+            if not is_safe_url(url=redirect_to, host=request.get_host()):
                 redirect_to = resolve_url(settings.LOGIN_REDIRECT_URL)
 
-            # Okay, security checks complete. Log the user in.
+            # Okay, security check complete. Log the user in.
             auth_login(request, form.get_user())
 
             if request.session.test_cookie_worked():
@@ -82,27 +76,27 @@ def logout(request, next_page=None,
     Logs out the user and displays 'You are logged out' message.
     """
     auth_logout(request)
-    redirect_to = request.REQUEST.get(redirect_field_name, '')
-    if redirect_to:
-        netloc = urlparse(redirect_to)[1]
+
+    if redirect_field_name in request.REQUEST:
+        next_page = request.REQUEST[redirect_field_name]
         # Security check -- don't allow redirection to a different host.
-        if not (netloc and netloc != request.get_host()):
-            return HttpResponseRedirect(redirect_to)
+        if not is_safe_url(url=next_page, host=request.get_host()):
+            next_page = request.path
 
-    if next_page is None:
-        current_site = get_current_site(request)
-        context = {
-            'site': current_site,
-            'site_name': current_site.name,
-            'title': _('Logged out')
-        }
-        if extra_context is not None:
-            context.update(extra_context)
-        return TemplateResponse(request, template_name, context,
-                                current_app=current_app)
-    else:
+    if next_page:
         # Redirect to this page until the session has been cleared.
-        return HttpResponseRedirect(next_page or request.path)
+        return HttpResponseRedirect(next_page)
+
+    current_site = get_current_site(request)
+    context = {
+        'site': current_site,
+        'site_name': current_site.name,
+        'title': _('Logged out')
+    }
+    if extra_context is not None:
+        context.update(extra_context)
+    return TemplateResponse(request, template_name, context,
+        current_app=current_app)
 
 
 def logout_then_login(request, login_url=None, current_app=None, extra_context=None):
diff --git a/django/contrib/comments/views/comments.py b/django/contrib/comments/views/comments.py
index 27d5a48ac6..7c02b21b6a 100644
--- a/django/contrib/comments/views/comments.py
+++ b/django/contrib/comments/views/comments.py
@@ -44,9 +44,6 @@ def post_comment(request, next=None, using=None):
         if not data.get('email', ''):
             data["email"] = request.user.email
 
-    # Check to see if the POST data overrides the view's next argument.
-    next = data.get("next", next)
-
     # Look up the object we're trying to comment about
     ctype = data.get("content_type")
     object_pk = data.get("object_pk")
@@ -100,7 +97,7 @@ def post_comment(request, next=None, using=None):
             template_list, {
                 "comment": form.data.get("comment", ""),
                 "form": form,
-                "next": next,
+                "next": data.get("next", next),
             },
             RequestContext(request, {})
         )
@@ -131,7 +128,8 @@ def post_comment(request, next=None, using=None):
         request=request
     )
 
-    return next_redirect(data, next, comment_done, c=comment._get_pk_val())
+    return next_redirect(request, fallback=next or 'comments-comment-done',
+        c=comment._get_pk_val())
 
 comment_done = confirmation_view(
     template="comments/posted.html",
diff --git a/django/contrib/comments/views/moderation.py b/django/contrib/comments/views/moderation.py
index 39933e75c8..31bb98fa63 100644
--- a/django/contrib/comments/views/moderation.py
+++ b/django/contrib/comments/views/moderation.py
@@ -10,7 +10,6 @@ from django.shortcuts import get_object_or_404, render_to_response
 from django.views.decorators.csrf import csrf_protect
 
 
-
 @csrf_protect
 @login_required
 def flag(request, comment_id, next=None):
@@ -27,7 +26,8 @@ def flag(request, comment_id, next=None):
     # Flag on POST
     if request.method == 'POST':
         perform_flag(request, comment)
-        return next_redirect(request.POST.copy(), next, flag_done, c=comment.pk)
+        return next_redirect(request, fallback=next or 'comments-flag-done',
+            c=comment.pk)
 
     # Render a form on GET
     else:
@@ -54,7 +54,8 @@ def delete(request, comment_id, next=None):
     if request.method == 'POST':
         # Flag the comment as deleted instead of actually deleting it.
         perform_delete(request, comment)
-        return next_redirect(request.POST.copy(), next, delete_done, c=comment.pk)
+        return next_redirect(request, fallback=next or 'comments-delete-done',
+            c=comment.pk)
 
     # Render a form on GET
     else:
@@ -81,7 +82,8 @@ def approve(request, comment_id, next=None):
     if request.method == 'POST':
         # Flag the comment as approved.
         perform_approve(request, comment)
-        return next_redirect(request.POST.copy(), next, approve_done, c=comment.pk)
+        return next_redirect(request, fallback=next or 'comments-approve-done',
+            c=comment.pk)
 
     # Render a form on GET
     else:
diff --git a/django/contrib/comments/views/utils.py b/django/contrib/comments/views/utils.py
index abaed68560..79f6376232 100644
--- a/django/contrib/comments/views/utils.py
+++ b/django/contrib/comments/views/utils.py
@@ -9,25 +9,26 @@ except ImportError:     # Python 2
     from urllib import urlencode
 
 from django.http import HttpResponseRedirect
-from django.core import urlresolvers
-from django.shortcuts import render_to_response
+from django.shortcuts import render_to_response, resolve_url
 from django.template import RequestContext
 from django.core.exceptions import ObjectDoesNotExist
 from django.contrib import comments
+from django.utils.http import is_safe_url
 
-def next_redirect(data, default, default_view, **get_kwargs):
+def next_redirect(request, fallback, **get_kwargs):
     """
     Handle the "where should I go next?" part of comment views.
 
-    The next value could be a kwarg to the function (``default``), or a
-    ``?next=...`` GET arg, or the URL of a given view (``default_view``). See
+    The next value could be a
+    ``?next=...`` GET arg or the URL of a given view (``fallback``). See
     the view modules for examples.
 
     Returns an ``HttpResponseRedirect``.
     """
-    next = data.get("next", default)
-    if next is None:
-        next = urlresolvers.reverse(default_view)
+    next = request.POST.get('next')
+    if not is_safe_url(url=next, host=request.get_host()):
+        next = resolve_url(fallback)
+
     if get_kwargs:
         if '#' in next:
             tmp = next.rsplit('#', 1)
diff --git a/django/utils/http.py b/django/utils/http.py
index 1c3b0039b5..0ab5198804 100644
--- a/django/utils/http.py
+++ b/django/utils/http.py
@@ -227,3 +227,15 @@ def same_origin(url1, url2):
     """
     p1, p2 = urllib_parse.urlparse(url1), urllib_parse.urlparse(url2)
     return (p1.scheme, p1.hostname, p1.port) == (p2.scheme, p2.hostname, p2.port)
+
+def is_safe_url(url, host=None):
+    """
+    Return ``True`` if the url is a safe redirection (i.e. it doesn't point to
+    a different host).
+
+    Always returns ``False`` on an empty url.
+    """
+    if not url:
+        return False
+    netloc = urllib_parse.urlparse(url)[1]
+    return not netloc or netloc == host
diff --git a/django/views/i18n.py b/django/views/i18n.py
index c1d456d667..c87e3a82db 100644
--- a/django/views/i18n.py
+++ b/django/views/i18n.py
@@ -9,6 +9,7 @@ from django.utils.text import javascript_quote
 from django.utils.encoding import smart_text
 from django.utils.formats import get_format_modules, get_format
 from django.utils._os import upath
+from django.utils.http import is_safe_url
 from django.utils import six
 
 def set_language(request):
@@ -22,11 +23,11 @@ def set_language(request):
     redirect to the page in the request (the 'next' parameter) without changing
     any state.
     """
-    next = request.REQUEST.get('next', None)
-    if not next:
-        next = request.META.get('HTTP_REFERER', None)
-    if not next:
-        next = '/'
+    next = request.REQUEST.get('next')
+    if not is_safe_url(url=next, host=request.get_host()):
+        next = request.META.get('HTTP_REFERER')
+        if not is_safe_url(url=next, host=request.get_host()):
+            next = '/'
     response = http.HttpResponseRedirect(next)
     if request.method == 'POST':
         lang_code = request.POST.get('language', None)