Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
add patch to fix CVE-2021-28861 (#3673)
- Loading branch information
Showing
2 changed files
with
122 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| From 2f09fe26b94ede913b151cb1bb3bcef543e113a2 Mon Sep 17 00:00:00 2001 | ||
| From: "Gregory P. Smith [Google]" <greg@krypto.org> | ||
| Date: Wed, 15 Jun 2022 20:10:15 +0000 | ||
| Subject: [PATCH 1/7] gh-87389: Fix an open redirection vulnerability in | ||
| http.server. | ||
|
|
||
| Fix an open redirection vulnerability in the `http.server` module when | ||
| an URI path starts with `//`. Vulnerability discovered, and initial fix | ||
| proposed, by Hamza Avvan. | ||
|
|
||
| Test authored and secondary mitigation by Gregory P. Smith [Google]. | ||
|
|
||
| diff -ruN a/Lib/http/server.py b/Lib/http/server.py | ||
| --- a/Lib/http/server.py 2022-03-16 06:27:21.000000000 -0700 | ||
| +++ b/Lib/http/server.py 2022-09-01 12:01:09.033232542 -0700 | ||
| @@ -330,6 +330,13 @@ | ||
| "Bad HTTP/0.9 request type (%r)" % command) | ||
| return False | ||
| self.command, self.path = command, path | ||
| + | ||
| + # gh-87389: The purpose of replacing '//' with '/' is to protect | ||
| + # against open redirect attacks possibly triggered if the path starts | ||
| + # with '//' because http clients treat //path as an absolute URI | ||
| + # without scheme (similar to http://path) rather than a path. | ||
| + if self.path.startswith('//'): | ||
| + self.path = '/' + self.path.lstrip('/') # Reduce to a single / | ||
|
|
||
| # Examine the headers and look for a Connection directive. | ||
| try: | ||
| diff -ruN a/Lib/test/test_httpservers.py b/Lib/test/test_httpservers.py | ||
| --- a/Lib/test/test_httpservers.py 2022-03-16 06:27:21.000000000 -0700 | ||
| +++ b/Lib/test/test_httpservers.py 2022-09-01 12:04:18.385086834 -0700 | ||
| @@ -329,7 +329,7 @@ | ||
| pass | ||
|
|
||
| def setUp(self): | ||
| - BaseTestCase.setUp(self) | ||
| + super().setUp() | ||
| self.cwd = os.getcwd() | ||
| basetempdir = tempfile.gettempdir() | ||
| os.chdir(basetempdir) | ||
| @@ -357,7 +357,7 @@ | ||
| except: | ||
| pass | ||
| finally: | ||
| - BaseTestCase.tearDown(self) | ||
| + super().tearDown() | ||
|
|
||
| def check_status_and_reason(self, response, status, data=None): | ||
| def close_conn(): | ||
| @@ -413,6 +413,53 @@ | ||
| self.check_status_and_reason(response, HTTPStatus.OK, | ||
| data=support.TESTFN_UNDECODABLE) | ||
|
|
||
| + def test_get_dir_redirect_location_domain_injection_bug(self): | ||
| + """Ensure //evil.co/..%2f../../X does not put //evil.co/ in Location. | ||
| + //netloc/ in a Location header is a redirect to a new host. | ||
| + https://github.com/python/cpython/issues/87389 | ||
| + This checks that a path resolving to a directory on our server cannot | ||
| + resolve into a redirect to another server. | ||
| + """ | ||
| + os.mkdir(os.path.join(self.tempdir, 'existing_directory')) | ||
| + url = f'/python.org/..%2f..%2f..%2f..%2f..%2f../%0a%0d/../{self.tempdir_name}/existing_directory' | ||
| + expected_location = f'{url}/' # /python.org.../ single slash single prefix, trailing slash | ||
| + # Canonicalizes to /tmp/tempdir_name/existing_directory which does | ||
| + # exist and is a dir, triggering the 301 redirect logic. | ||
| + response = self.request(url) | ||
| + self.check_status_and_reason(response, HTTPStatus.MOVED_PERMANENTLY) | ||
| + location = response.getheader('Location') | ||
| + self.assertEqual(location, expected_location, msg='non-attack failed!') | ||
| + | ||
| + # //python.org... multi-slash prefix, no trailing slash | ||
| + attack_url = f'/{url}' | ||
| + response = self.request(attack_url) | ||
| + self.check_status_and_reason(response, HTTPStatus.MOVED_PERMANENTLY) | ||
| + location = response.getheader('Location') | ||
| + self.assertFalse(location.startswith('//'), msg=location) | ||
| + self.assertEqual(location, expected_location, | ||
| + msg='Expected Location header to start with a single / and ' | ||
| + 'end with a / as this is a directory redirect.') | ||
| + | ||
| + # ///python.org... triple-slash prefix, no trailing slash | ||
| + attack3_url = f'//{url}' | ||
| + response = self.request(attack3_url) | ||
| + self.check_status_and_reason(response, HTTPStatus.MOVED_PERMANENTLY) | ||
| + self.assertEqual(response.getheader('Location'), expected_location) | ||
| + | ||
| + # If the second word in the http request (Request-URI for the http | ||
| + # method) is a full URI, we don't worry about it, as that'll be parsed | ||
| + # and reassembled as a full URI within BaseHTTPRequestHandler.send_head | ||
| + # so no errant scheme-less //netloc//evil.co/ domain mixup can happen. | ||
| + attack_scheme_netloc_2slash_url = f'https://pypi.org/{url}' | ||
| + expected_scheme_netloc_location = f'{attack_scheme_netloc_2slash_url}/' | ||
| + response = self.request(attack_scheme_netloc_2slash_url) | ||
| + self.check_status_and_reason(response, HTTPStatus.MOVED_PERMANENTLY) | ||
| + location = response.getheader('Location') | ||
| + # We're just ensuring that the scheme and domain make it through, if | ||
| + # there are or aren't multiple slashes at the start of the path that | ||
| + # follows that isn't important in this Location: header. | ||
| + self.assertTrue(location.startswith('https://pypi.org/'), msg=location) | ||
| + | ||
| def test_get(self): | ||
| #constructs the path relative to the root directory of the HTTPServer | ||
| response = self.request(self.base_url + '/test') | ||
| diff -ruN a/Misc/NEWS b/Misc/NEWS | ||
| --- a/Misc/NEWS 2022-03-16 07:04:10.000000000 -0700 | ||
| +++ b/Misc/NEWS 2022-09-01 14:01:37.138915508 -0700 | ||
| @@ -52,6 +52,9 @@ | ||
|
|
||
| Security | ||
| -------- | ||
| +- bpo-43223: mod:`http.server`: Fix an open redirection vulnerability in | ||
| + the HTTP server when an URL contains ``//``. | ||
| + Vulnerability discovered and fixed by Hamza Avvan. | ||
|
|
||
| - bpo-44394: Update the vendored copy of libexpat to 2.4.1 (from 2.2.8) to | ||
| get the fix for the CVE-2013-0340 "Billion Laughs" vulnerability. This |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters