Fix an open redirect vulnerability

cyface · Oct 7, 2021 · 0a0f7ac · 0a0f7ac
1 parent 0ccdfd7
commit 0a0f7ac
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 0 deletions.
diff --git a/termsandconditions/tests.py b/termsandconditions/tests.py
@@ -242,6 +242,32 @@ def test_accept(self):
         )
         self.assertContains(accept_version_post_response, "Secure")
 
+    def test_accept_redirect_safe(self):
+        # Pre-accept terms 2 and 3
+        UserTermsAndConditions.objects.create(user=self.user1, terms=self.terms2)
+        UserTermsAndConditions.objects.create(user=self.user1, terms=self.terms3)
+
+        LOGGER.debug("Test user1 login for test_accept_redirect")
+        login_response = self.client.login(username="user1", password="user1password")
+        self.assertTrue(login_response)
+
+        LOGGER.debug("Test /terms/accept/site-terms/1/ post")
+        accept_response = self.client.post(
+            "/terms/accept/", {"terms": 1, "returnTo": "/secure/"}, follow=True
+        )
+        self.assertRedirects(accept_response, "/secure/")
+
+    def test_accept_redirect_unsafe(self):
+        # Pre-accept terms 2 and 3
+        UserTermsAndConditions.objects.create(user=self.user1, terms=self.terms2)
+        UserTermsAndConditions.objects.create(user=self.user1, terms=self.terms3)
+
+        LOGGER.debug("Test /terms/accept/contrib-terms/3/ post")
+        accept_response = self.client.post(
+            "/terms/accept/", {"terms": 3, "returnTo": "http://attacker/"}, follow=False
+        )
+        self.assertRedirects(accept_response, "/")
+
     def test_accept_store_ip_address(self):
         """Test with IP address storage setting true (default)"""
         self.client.login(username="user1", password="user1password")

diff --git a/termsandconditions/views.py b/termsandconditions/views.py
@@ -1,4 +1,5 @@
 """Django Views for the termsandconditions module"""
+from urllib.parse import urlparse
 
 # pylint: disable=E1120,R0901,R0904
 from django.contrib.auth.models import User
@@ -79,6 +80,11 @@ def post(self, request, *args, **kwargs):
         return_url = request.POST.get("returnTo", "/")
         terms_ids = request.POST.getlist("terms")
 
+        parsed = urlparse(return_url)
+        if parsed.hostname and parsed.hostname not in settings.ALLOWED_HOSTS:
+            # Make sure the return url is a relative path or a trusted hostname
+            return_url = '/'
+
         if not terms_ids:  # pragma: nocover
             return HttpResponseRedirect(return_url)