Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion webapp/graphite/url_shortener/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from django.urls import reverse
from django.shortcuts import get_object_or_404
from django.http import HttpResponse, HttpResponsePermanentRedirect
from django.utils.http import url_has_allowed_host_and_scheme

from graphite.url_shortener.baseconv import base62
from graphite.url_shortener.models import Link
Expand All @@ -14,9 +15,15 @@ def follow(request, link_id):
"""Follow existing links"""
key = base62.to_decimal(link_id)
link = get_object_or_404(Link, pk=key)
browser_url = reverse('browser')
# Strip leading slashes from the stored URL to prevent open redirect via
# protocol-relative URLs (e.g. //evil.com) being used as redirect targets.
url = reverse('browser') + link.url.lstrip('/')
url = browser_url + link.url.lstrip('/')
# Validate the resulting URL is safe and does not redirect to an external
# domain (e.g. via backslash bypass: /\evil.com interpreted as //evil.com
# by some browsers).
if not url_has_allowed_host_and_scheme(url=url, allowed_hosts={request.get_host()}):
url = browser_url
return HttpResponsePermanentRedirect(url)


Expand Down
22 changes: 22 additions & 0 deletions webapp/tests/test_url_shortener.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

from urllib.parse import urlparse

from django.utils.http import url_has_allowed_host_and_scheme

from .base import TestCase


Expand Down Expand Up @@ -43,3 +45,23 @@ def test_follow_open_redirect_prevention(self):
parsed = urlparse(redirect_url)
self.assertFalse(bool(parsed.netloc),
'Open redirect to external domain detected: %s' % redirect_url)

def test_follow_open_redirect_backslash_prevention(self):
"""Test that backslash-prefixed URLs cannot cause open redirects.

Some browsers interpret /\\evil.com as //evil.com (a protocol-relative
URL pointing to evil.com). The follow() view must reject such URLs.
"""
shorten_url = reverse('shorten', kwargs={'path': '\\evil.com'})
response = self.client.get(shorten_url)
self.assertEqual(response.status_code, 200)
short_path = response.content.decode('utf-8')

follow_response = self.client.get(short_path)
self.assertEqual(follow_response.status_code, 301)
redirect_url = follow_response['Location']
# The redirect must be a safe internal URL
self.assertTrue(
url_has_allowed_host_and_scheme(url=redirect_url, allowed_hosts={'testserver'}),
'Unsafe redirect detected: %s' % redirect_url,
)
Loading