From a4875a2dc411ecd775ca5b89c6310b5a20d7ece3 Mon Sep 17 00:00:00 2001 From: Katsuhiko YOSHIDA Date: Sat, 22 Dec 2018 22:37:51 +0900 Subject: [PATCH 1/2] bpo-33661: Clear Authorization header when redirect to cross-site --- Lib/test/test_urllib2.py | 26 ++++++++++++++++++++++++++ Lib/urllib/request.py | 9 ++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py index 876fcd4199fb92..89fa757db43c38 100644 --- a/Lib/test/test_urllib2.py +++ b/Lib/test/test_urllib2.py @@ -1306,6 +1306,32 @@ def request(conn, method, url, *pos, **kw): fp = urllib.request.urlopen("http://python.org/path") self.assertEqual(fp.geturl(), "http://python.org/path?query") + def test_redirect_to_cross_domain_with_authorization_header(self): + from_url = "http://example.com/index.html" + to_url = "http://cracker.com/index.html" + h = urllib.request.HTTPRedirectHandler() + o = h.parent = MockOpener() + req = Request(from_url) + req.add_header("Authorization", "Basic: xxxx") + req.timeout = socket._GLOBAL_DEFAULT_TIMEOUT + h.http_error_302(req, MockFile(), 302, "", + MockHeaders({"location": to_url})) + + self.assertNotIn("Authorization", o.req.headers) + + def test_redirect_to_same_domain_with_authorization_header(self): + from_url = "http://example.com/index.html" + to_url = "http://example.com/index.html" + h = urllib.request.HTTPRedirectHandler() + o = h.parent = MockOpener() + req = Request(from_url) + req.add_header("Authorization", "Basic: xxxx") + req.timeout = socket._GLOBAL_DEFAULT_TIMEOUT + h.http_error_302(req, MockFile(), 302, "", + MockHeaders({"location": to_url})) + + self.assertIn("Authorization", o.req.headers) + def test_redirect_encoding(self): # Some characters in the redirect target may need special handling, # but most ASCII characters should be treated as already encoded diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py index 9a3d399f018931..6a6ab35963e1a6 100644 --- a/Lib/urllib/request.py +++ b/Lib/urllib/request.py @@ -685,11 +685,18 @@ def redirect_request(self, req, fp, code, msg, headers, newurl): CONTENT_HEADERS = ("content-length", "content-type") newheaders = {k: v for k, v in req.headers.items() if k.lower() not in CONTENT_HEADERS} - return Request(newurl, + newrequest = Request(newurl, headers=newheaders, origin_req_host=req.origin_req_host, unverifiable=True) + if newrequest.host != req.host: + for k in newheaders.keys(): + if "authorization" == k.lower(): + del newrequest.headers[k] + + return newrequest + # Implementation note: To avoid the server sending us into an # infinite loop, the request object needs to track what URLs we # have already seen. Do this by adding a handler-specific From 9068ef01bec5e81c510cb05721c7f4a29951e796 Mon Sep 17 00:00:00 2001 From: Katsuhiko YOSHIDA Date: Sat, 29 Dec 2018 23:51:10 +0900 Subject: [PATCH 2/2] bpo-33661: Clear Cookie header when redirect to cross-site --- Lib/test/test_urllib2.py | 12 ++++++++---- Lib/urllib/request.py | 6 +++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py index 89fa757db43c38..1935f9df2d0585 100644 --- a/Lib/test/test_urllib2.py +++ b/Lib/test/test_urllib2.py @@ -1306,31 +1306,35 @@ def request(conn, method, url, *pos, **kw): fp = urllib.request.urlopen("http://python.org/path") self.assertEqual(fp.geturl(), "http://python.org/path?query") - def test_redirect_to_cross_domain_with_authorization_header(self): + def test_redirect_to_cross_domain_with_sensitive_header(self): from_url = "http://example.com/index.html" to_url = "http://cracker.com/index.html" h = urllib.request.HTTPRedirectHandler() o = h.parent = MockOpener() req = Request(from_url) - req.add_header("Authorization", "Basic: xxxx") + req.add_header("Authorization", "Basic foo") + req.add_header("Cookie", "bar") req.timeout = socket._GLOBAL_DEFAULT_TIMEOUT h.http_error_302(req, MockFile(), 302, "", MockHeaders({"location": to_url})) self.assertNotIn("Authorization", o.req.headers) + self.assertNotIn("Cookie", o.req.headers) - def test_redirect_to_same_domain_with_authorization_header(self): + def test_redirect_to_same_domain_with_sensitive_header(self): from_url = "http://example.com/index.html" to_url = "http://example.com/index.html" h = urllib.request.HTTPRedirectHandler() o = h.parent = MockOpener() req = Request(from_url) - req.add_header("Authorization", "Basic: xxxx") + req.add_header("Authorization", "Basic foo") + req.add_header("Cookie", "bar") req.timeout = socket._GLOBAL_DEFAULT_TIMEOUT h.http_error_302(req, MockFile(), 302, "", MockHeaders({"location": to_url})) self.assertIn("Authorization", o.req.headers) + self.assertIn("Cookie", o.req.headers) def test_redirect_encoding(self): # Some characters in the redirect target may need special handling, diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py index 6a6ab35963e1a6..d37fc5d830856b 100644 --- a/Lib/urllib/request.py +++ b/Lib/urllib/request.py @@ -690,10 +690,10 @@ def redirect_request(self, req, fp, code, msg, headers, newurl): origin_req_host=req.origin_req_host, unverifiable=True) + SENSITIVE_HEADERS = ("authorization", "cookie") if newrequest.host != req.host: - for k in newheaders.keys(): - if "authorization" == k.lower(): - del newrequest.headers[k] + newrequest.headers = {k: v for k, v in newrequest.headers.items() + if k.lower() not in SENSITIVE_HEADERS} return newrequest