Merge pull request #2906 from graphite-project/copilot/fix-issue-2872-again

Fix open redirect vulnerability in URL shortener via backslash bypass

Author: deniszh

webapp/graphite/url_shortener/views.py

@@ -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
@@ -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)
 
 

webapp/tests/test_url_shortener.py

@@ -5,6 +5,8 @@
 
 from urllib.parse import urlparse
 
+from django.utils.http import url_has_allowed_host_and_scheme
+
 from .base import TestCase
 
 
@@ -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,
+        )