Skip to content

Commit 1abd951

Browse files
minrkkevin-bates
authored andcommitted
Validate redirect target in TrailingSlashHandler
Fixes open redirect vulnerability GHSA-c7vm-f5p4-8fqh
1 parent 5a73a8e commit 1abd951

File tree

2 files changed

+28
-4
lines changed

2 files changed

+28
-4
lines changed

notebook/base/handlers.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -853,13 +853,18 @@ def get(self):
853853

854854
class TrailingSlashHandler(web.RequestHandler):
855855
"""Simple redirect handler that strips trailing slashes
856-
856+
857857
This should be the first, highest priority handler.
858858
"""
859-
859+
860860
def get(self):
861-
self.redirect(self.request.uri.rstrip('/'))
862-
861+
path, *rest = self.request.uri.partition("?")
862+
# trim trailing *and* leading /
863+
# to avoid misinterpreting repeated '//'
864+
path = "/" + path.strip("/")
865+
new_uri = "".join([path, *rest])
866+
self.redirect(new_uri)
867+
863868
post = put = get
864869

865870

@@ -910,6 +915,7 @@ def get(self):
910915
url = sep.join([self._url, self.request.query])
911916
self.redirect(url, permanent=self._permanent)
912917

918+
913919
class PrometheusMetricsHandler(IPythonHandler):
914920
"""
915921
Return prometheus metrics for this notebook server

notebook/tests/test_paths.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@
22
import re
33

44
from notebook.base.handlers import path_regex
5+
from notebook.utils import url_path_join
6+
from .launchnotebook import NotebookTestBase
57

68
# build regexps that tornado uses:
79
path_pat = re.compile('^' + '/x%s' % path_regex + '$')
810

11+
912
def test_path_regex():
1013
for path in (
1114
'/x',
@@ -29,3 +32,18 @@ def test_path_regex_bad():
2932
'/y/x/foo',
3033
):
3134
assert not re.match(path_pat, path)
35+
36+
37+
class RedirectTestCase(NotebookTestBase):
38+
def test_trailing_slash(self):
39+
for uri, expected in (
40+
("/notebooks/mynotebook/", "/notebooks/mynotebook"),
41+
("////foo///", "/foo"),
42+
("//example.com/", "/example.com"),
43+
("/has/param/?hasparam=true", "/has/param?hasparam=true"),
44+
):
45+
r = self.request("GET", uri, allow_redirects=False)
46+
print(uri, expected)
47+
assert r.status_code == 302
48+
assert "Location" in r.headers
49+
assert r.headers["Location"] == url_path_join(self.url_prefix, expected)

0 commit comments

Comments
 (0)