Merge branch 'check-redirect-target'

Author: minrk

docs/source/changelog.rst

@@ -31,21 +31,21 @@ We strongly recommend that you upgrade pip to version 9+ of pip before upgrading
 - Further improve compatibility with tornado 6 with improved
   checks for when websockets are closed.
 - Fix regression in 5.7.6 on Windows where .js files could have the wrong mime-type.
+- Fix Open Redirect vulnerability where certain malicious URLs could redirect from the Jupyter login page to a malicious site after a successful login. A CVE has been requested for this vulnerability.
 
 .. _release-5.7.6:
 
 5.7.6
 -----
 
-5.7.6 contains a security fix for a cross-site inclusion (XSSI) vulnerability,
+5.7.6 contains a security fix for a cross-site inclusion (XSSI) vulnerability (CVE-2019–9644),
 where files at a known URL could be included in a page from an unauthorized website if the user is logged into a Jupyter server.
 The fix involves setting the ``X-Content-Type-Options: nosniff``
 header, and applying CSRF checks previously on all non-GET
 API requests to GET requests to API endpoints and the /files/ endpoint.
 
 The attacking page is able to access some contents of files when using Internet Explorer through script errors,
 but this has not been demonstrated with other browsers.
-A CVE has been requested for this vulnerability.
 
 .. _release-5.7.5:
 

notebook/auth/login.py

@@ -39,14 +39,19 @@ def _redirect_safe(self, url, default=None):
         """
         if default is None:
             default = self.base_url
-        if not url.startswith(self.base_url):
+        # protect chrome users from mishandling unescaped backslashes.
+        # \ is not valid in urls, but some browsers treat it as /
+        # instead of %5C, causing `\\` to behave as `//`
+        url = url.replace("\\", "%5C")
+        parsed = urlparse(url)
+        if parsed.netloc or not (parsed.path + '/').startswith(self.base_url):
             # require that next_url be absolute path within our path
             allow = False
             # OR pass our cross-origin check
-            if '://' in url:
+            if parsed.netloc:
                 # if full URL, run our cross-origin check:
-                parsed = urlparse(url.lower())
                 origin = '%s://%s' % (parsed.scheme, parsed.netloc)
+                origin = origin.lower()
                 if self.allow_origin:
                     allow = self.allow_origin == origin
                 elif self.allow_origin_pat:

notebook/auth/tests/test_login.py

@@ -0,0 +1,48 @@
+"""Tests for login redirects"""
+
+import requests
+from tornado.httputil import url_concat
+
+from notebook.tests.launchnotebook import NotebookTestBase
+
+
+class LoginTest(NotebookTestBase):
+    def login(self, next):
+        first = requests.get(self.base_url() + "login")
+        first.raise_for_status()
+        resp = requests.post(
+            url_concat(
+                self.base_url() + "login",
+                {'next': next},
+            ),
+            allow_redirects=False,
+            data={
+                "password": self.token,
+                "_xsrf": first.cookies.get("_xsrf", ""),
+            },
+            cookies=first.cookies,
+        )
+        resp.raise_for_status()
+        return resp.headers['Location']
+
+    def test_next_bad(self):
+        for bad_next in (
+            "//some-host",
+            "//host" + self.url_prefix + "tree",
+            "https://google.com",
+            "/absolute/not/base_url",
+        ):
+            url = self.login(next=bad_next)
+            self.assertEqual(url, self.url_prefix)
+        assert url
+
+    def test_next_ok(self):
+        for next_path in (
+            "tree/",
+            "//" + self.url_prefix + "tree",
+            "notebooks/notebook.ipynb",
+            "tree//something",
+        ):
+            expected = self.url_prefix + next_path
+            actual = self.login(next=expected)
+            self.assertEqual(actual, expected)