Skip to content
This repository was archived by the owner on Jan 3, 2024. It is now read-only.

Commit a915dbd

Browse files
Julian BermanJulian Berman
Julian Berman
authored and
Julian Berman
committed
Pull in the http.server vulnerability fix from python/cpython#87389
Fixes an open redirection vulnerability for paths starting with `//`. Closes: #3812
1 parent 15f0b1f commit a915dbd

File tree

2 files changed

+56
-0
lines changed

2 files changed

+56
-0
lines changed

lib-python/3/http/server.py

+7
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,13 @@ def parse_request(self):
332332
return False
333333
self.command, self.path = command, path
334334

335+
# gh-87389: The purpose of replacing '//' with '/' is to protect
336+
# against open redirect attacks possibly triggered if the path starts
337+
# with '//' because http clients treat //path as an absolute URI
338+
# without scheme (similar to http://path) rather than a path.
339+
if self.path.startswith('//'):
340+
self.path = '/' + self.path.lstrip('/') # Reduce to a single /
341+
335342
# Examine the headers and look for a Connection directive.
336343
try:
337344
self.headers = http.client.parse_headers(self.rfile,

lib-python/3/test/test_httpservers.py

+49
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,55 @@ def test_undecodable_filename(self):
416416
self.check_status_and_reason(response, HTTPStatus.OK,
417417
data=support.TESTFN_UNDECODABLE)
418418

419+
def test_get_dir_redirect_location_domain_injection_bug(self):
420+
"""Ensure //evil.co/..%2f../../X does not put //evil.co/ in Location.
421+
422+
//netloc/ in a Location header is a redirect to a new host.
423+
https://github.com/python/cpython/issues/87389
424+
425+
This checks that a path resolving to a directory on our server cannot
426+
resolve into a redirect to another server.
427+
"""
428+
os.mkdir(os.path.join(self.tempdir, 'existing_directory'))
429+
url = f'/python.org/..%2f..%2f..%2f..%2f..%2f../%0a%0d/../{self.tempdir_name}/existing_directory'
430+
expected_location = f'{url}/' # /python.org.../ single slash single prefix, trailing slash
431+
# Canonicalizes to /tmp/tempdir_name/existing_directory which does
432+
# exist and is a dir, triggering the 301 redirect logic.
433+
response = self.request(url)
434+
self.check_status_and_reason(response, HTTPStatus.MOVED_PERMANENTLY)
435+
location = response.getheader('Location')
436+
self.assertEqual(location, expected_location, msg='non-attack failed!')
437+
438+
# //python.org... multi-slash prefix, no trailing slash
439+
attack_url = f'/{url}'
440+
response = self.request(attack_url)
441+
self.check_status_and_reason(response, HTTPStatus.MOVED_PERMANENTLY)
442+
location = response.getheader('Location')
443+
self.assertFalse(location.startswith('//'), msg=location)
444+
self.assertEqual(location, expected_location,
445+
msg='Expected Location header to start with a single / and '
446+
'end with a / as this is a directory redirect.')
447+
448+
# ///python.org... triple-slash prefix, no trailing slash
449+
attack3_url = f'//{url}'
450+
response = self.request(attack3_url)
451+
self.check_status_and_reason(response, HTTPStatus.MOVED_PERMANENTLY)
452+
self.assertEqual(response.getheader('Location'), expected_location)
453+
454+
# If the second word in the http request (Request-URI for the http
455+
# method) is a full URI, we don't worry about it, as that'll be parsed
456+
# and reassembled as a full URI within BaseHTTPRequestHandler.send_head
457+
# so no errant scheme-less //netloc//evil.co/ domain mixup can happen.
458+
attack_scheme_netloc_2slash_url = f'https://pypi.org/{url}'
459+
expected_scheme_netloc_location = f'{attack_scheme_netloc_2slash_url}/'
460+
response = self.request(attack_scheme_netloc_2slash_url)
461+
self.check_status_and_reason(response, HTTPStatus.MOVED_PERMANENTLY)
462+
location = response.getheader('Location')
463+
# We're just ensuring that the scheme and domain make it through, if
464+
# there are or aren't multiple slashes at the start of the path that
465+
# follows that isn't important in this Location: header.
466+
self.assertTrue(location.startswith('https://pypi.org/'), msg=location)
467+
419468
def test_get(self):
420469
#constructs the path relative to the root directory of the HTTPServer
421470
response = self.request(self.base_url + '/test')

0 commit comments

Comments
 (0)