CVE-2021-21337: URL Redirection to Untrusted Site ('Open Redirect') (CWE-601)
value: Products.PluggableAuthService is a pluggable Zope authentication and authorization framework. In Products.PluggableAuthService before version 2.6.0 there is an open redirect vulnerability. A maliciously crafted link to the login form and login functionality could redirect the browser to a different website. The problem has been fixed in version 2.6.1. Depending on how you have installed Products.PluggableAuthService, you should change the buildout version pin to `2.6.1` and re-run the buildout, or if you used `pip` simply do `pip install "Products.PluggableAuthService>=2.6.1".
CWE description: A web application accepts a user-controlled input that specifies a link to an external site, and uses that link in a Redirect. This simplifies phishing attacks.
CVSS v3 base score: 6.1
Published: 2021-03-08T21:15Z
Repository: https://github.com/zopefoundation/Products.PluggableAuthService
Language: Python
Files changed: CHANGES.rst, src/Products/PluggableAuthService/plugins/CookieAuthHelper.py, src/Products/PluggableAuthService/plugins/tests/test_CookieAuthHelper.py
Commit date: 2021-02-23 12:23:37 +0100
Commit message: - Fix open redirect issue in  redirect handling

Diff: diff --git a/CHANGES.rst b/CHANGES.rst
index 0a3e4a2..d0683af 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -2,10 +2,12 @@ Change Log
 ==========
 
 2.6.0 (unreleased)
 ------------------
 
+- Fix open redirect issue in `Cookie Auth Helper` redirect handling
+
 - Add support for Python 3.9.
 
 
 2.5.1 (2020-11-13)
 ------------------
diff --git a/src/Products/PluggableAuthService/plugins/CookieAuthHelper.py b/src/Products/PluggableAuthService/plugins/CookieAuthHelper.py
index 6d5f140..af86b0b 100644
--- a/src/Products/PluggableAuthService/plugins/CookieAuthHelper.py
+++ b/src/Products/PluggableAuthService/plugins/CookieAuthHelper.py
@@ -26,10 +26,12 @@ from binascii import Error
 from binascii import hexlify
 
 import six
 from six.moves.urllib.parse import quote
 from six.moves.urllib.parse import unquote
+from six.moves.urllib.parse import urlparse
+from six.moves.urllib.parse import urlunparse
 
 from AccessControl.class_init import InitializeClass
 from AccessControl.Permissions import view
 from AccessControl.SecurityInfo import ClassSecurityInfo
 from OFS.Folder import Folder
@@ -223,10 +225,16 @@ class CookieAuthHelper(Folder, BasePlugin):
                     # it might be protected itself due to misconfiguration -
                     # the only sane thing to do is to give up because we are
                     # in an endless redirect loop.
                     return 0
 
+            # Sanitize the return URL ``came_from`` and only allow local URLs
+            # to prevent an open exploitable redirect issue
+            if came_from:
+                parsed = urlparse(came_from)
+                came_from = urlunparse(('', '') + parsed[2:])
+
             if '?' in url:
                 sep = '&'
             else:
                 sep = '?'
             url = '%s%scame_from=%s' % (url, sep, quote(came_from))
diff --git a/src/Products/PluggableAuthService/plugins/tests/test_CookieAuthHelper.py b/src/Products/PluggableAuthService/plugins/tests/test_CookieAuthHelper.py
index 20888a2..2fb9e3a 100644
--- a/src/Products/PluggableAuthService/plugins/tests/test_CookieAuthHelper.py
+++ b/src/Products/PluggableAuthService/plugins/tests/test_CookieAuthHelper.py
@@ -126,21 +126,24 @@ class CookieAuthHelperTests(unittest.TestCase,
         self.assertEqual(helper.extractCredentials(request), {})
 
     def test_challenge(self):
         rc, root, folder, object = self._makeTree()
         response = FauxCookieResponse()
-        testURL = 'http://test'
+        testPath = '/some/path'
+        testURL = 'http://test' + testPath
         request = FauxRequest(RESPONSE=response, URL=testURL,
                               ACTUAL_URL=testURL)
         root.REQUEST = request
 
         helper = self._makeOne().__of__(root)
 
         helper.challenge(request, response)
         self.assertEqual(response.status, 302)
         self.assertEqual(len(response.headers), 3)
-        self.assertTrue(response.headers['Location'].endswith(quote(testURL)))
+        loc = response.headers['Location']
+        self.assertTrue(loc.endswith(quote(testPath)))
+        self.assertNotIn(testURL, loc)
         self.assertEqual(response.headers['Cache-Control'], 'no-cache')
         self.assertEqual(response.headers['Expires'],
                          'Sat, 01 Jan 2000 00:00:00 GMT')
 
     def test_challenge_with_vhm(self):
@@ -157,12 +160,13 @@ class CookieAuthHelperTests(unittest.TestCase,
 
         helper.challenge(request, response)
         self.assertEqual(response.status, 302)
         self.assertEqual(len(response.headers), 3)
         loc = response.headers['Location']
-        self.assertTrue(loc.endswith(quote(actualURL)))
+        self.assertTrue(loc.endswith(quote('/xxx')))
         self.assertFalse(loc.endswith(quote(vhm)))
+        self.assertNotIn(actualURL, loc)
         self.assertEqual(response.headers['Cache-Control'], 'no-cache')
         self.assertEqual(response.headers['Expires'],
                          'Sat, 01 Jan 2000 00:00:00 GMT')
 
     def test_resetCredentials(self):

Vulnerable code:         testURL = 'http://test'
        self.assertTrue(response.headers['Location'].endswith(quote(testURL)))
        self.assertTrue(loc.endswith(quote(actualURL)))
Fixed code: - Fix open redirect issue in `Cookie Auth Helper` redirect handling

from six.moves.urllib.parse import urlparse
from six.moves.urllib.parse import urlunparse
            # Sanitize the return URL ``came_from`` and only allow local URLs
            # to prevent an open exploitable redirect issue
            if came_from:
                parsed = urlparse(came_from)
                came_from = urlunparse(('', '') + parsed[2:])

        testPath = '/some/path'
        testURL = 'http://test' + testPath
        loc = response.headers['Location']
        self.assertTrue(loc.endswith(quote(testPath)))
        self.assertNotIn(testURL, loc)
        self.assertTrue(loc.endswith(quote('/xxx')))
        self.assertNotIn(actualURL, loc)