Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

SECURITY FIX: fixed default_redirect to prevent XSS attack

Discovered in django.contrib.auth it is possible for an attack to craft a
specific URL using ?next to execute arbitrary Javascript within the victim's
browser.

We now only redirect if the URL scheme is allowed and it matches the allowed
host (based off of request.get_host() by default.) You are able to pass in
custom overrides to these options using get_success_url methods.
  • Loading branch information...
commit f3b2c442fb1263ec625ed5f9a6f10cea56a6c798 1 parent 329b31d
@brosner brosner authored
Showing with 40 additions and 19 deletions.
  1. +30 −9 account/utils.py
  2. +10 −10 account/views.py
View
39 account/utils.py
@@ -1,10 +1,11 @@
+import functools
import hashlib
import importlib
import random
import urlparse
from django.core import urlresolvers
-from django.core.exceptions import ImproperlyConfigured
+from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation
from django.http import HttpResponseRedirect, QueryDict
from account.conf import settings
@@ -18,14 +19,16 @@ def default_redirect(request, fallback_url, **kwargs):
if hasattr(request, "session"):
session_key_value = kwargs.get("session_key_value", "redirect_to")
next = request.session.get(session_key_value)
- if next:
- netloc = urlparse.urlparse(next)[1]
- redirect_to = next
- # security check -- don't allow redirection to a different host
- if netloc and netloc != request.get_host():
- redirect_to = fallback_url
- else:
- redirect_to = fallback_url
+ is_safe = functools.partial(
+ ensure_safe_url,
+ allowed_protocols=kwargs.get("allowed_protocols"),
+ allowed_host=request.get_host()
+ )
+ redirect_to = next if next and is_safe(next) else fallback_url
+ # perform one last check to ensure the URL is safe to redirect to. if it
+ # is not then we should bail here as it is likely developer error and
+ # they should be notified
+ is_safe(redirect_to, raise_on_fail=True)
return redirect_to
@@ -33,6 +36,24 @@ def user_display(user):
return settings.ACCOUNT_USER_DISPLAY(user)
+def ensure_safe_url(url, allowed_protocols=None, allowed_host=None, raise_on_fail=False):
+ if allowed_protocols is None:
+ allowed_protocols = ["http", "https"]
+ parsed = urlparse.urlparse(url)
+ # perform security checks to ensure no malicious intent
+ # (i.e., an XSS attack with a data URL)
+ safe = True
+ if parsed.scheme and parsed.scheme not in allowed_protocols:
+ if raise_on_fail:
+ raise SuspiciousOperation("Unsafe redirect to URL with protocol '%s'" % parsed.scheme)
+ safe = False
+ if allowed_host and parsed.netloc and parsed.netloc != allowed_host:
+ if raise_on_fail:
+ raise SuspiciousOperation("Unsafe redirect to URL not matching host '%s'" % allowed_host)
+ safe = False
+ return safe
+
+
def random_token(extra=None, hash_func=hashlib.sha256):
if extra is None:
extra = []
View
20 account/views.py
@@ -148,10 +148,10 @@ def form_valid(self, form):
)
return redirect(self.get_success_url())
- def get_success_url(self, fallback_url=None):
+ def get_success_url(self, fallback_url=None, **kwargs):
if fallback_url is None:
fallback_url = settings.ACCOUNT_SIGNUP_REDIRECT_URL
- return default_redirect(self.request, fallback_url)
+ return default_redirect(self.request, fallback_url, **kwargs)
def get_redirect_field_name(self):
return self.redirect_field_name
@@ -261,10 +261,10 @@ def form_valid(self, form):
def after_login(self, form):
signals.user_logged_in.send(sender=LoginView, user=form.user, form=form)
- def get_success_url(self, fallback_url=None):
+ def get_success_url(self, fallback_url=None, **kwargs):
if fallback_url is None:
fallback_url = settings.ACCOUNT_LOGIN_REDIRECT_URL
- return default_redirect(self.request, fallback_url)
+ return default_redirect(self.request, fallback_url, **kwargs)
def get_redirect_field_name(self):
return self.redirect_field_name
@@ -415,10 +415,10 @@ def form_valid(self, form):
self.change_password(form)
return redirect(self.get_success_url())
- def get_success_url(self, fallback_url=None):
+ def get_success_url(self, fallback_url=None, **kwargs):
if fallback_url is None:
fallback_url = settings.ACCOUNT_PASSWORD_CHANGE_REDIRECT_URL
- return default_redirect(self.request, fallback_url)
+ return default_redirect(self.request, fallback_url, **kwargs)
def send_email(self, user):
protocol = getattr(settings, "DEFAULT_HTTP_PROTOCOL", "http")
@@ -522,10 +522,10 @@ def form_valid(self, form):
)
return redirect(self.get_success_url())
- def get_success_url(self, fallback_url=None):
+ def get_success_url(self, fallback_url=None, **kwargs):
if fallback_url is None:
fallback_url = settings.ACCOUNT_PASSWORD_RESET_REDIRECT_URL
- return default_redirect(self.request, fallback_url)
+ return default_redirect(self.request, fallback_url, **kwargs)
def get_user(self):
try:
@@ -612,10 +612,10 @@ def update_account(self, form):
setattr(account, k, v)
account.save()
- def get_success_url(self, fallback_url=None):
+ def get_success_url(self, fallback_url=None, **kwargs):
if fallback_url is None:
fallback_url = settings.ACCOUNT_SETTINGS_REDIRECT_URL
- return default_redirect(self.request, fallback_url)
+ return default_redirect(self.request, fallback_url, **kwargs)
class DeleteView(LogoutView):
Please sign in to comment.
Something went wrong with that request. Please try again.