address open redirect with 3 forward slashes

Ie36401c782f023d1d5f2623732619105dc2cfa24 was intended
to address OSSA-2021-002 (CVE-2021-3654) however after its
release it was discovered that the fix only worked
for urls with 2 leading slashes or more then 4.

This change adresses the missing edgecase for 3 leading slashes
and also maintian support for rejecting 2+.

Change-Id: I95f68be76330ff09e5eabb5ef8dd9a18f5547866
co-authored-by: Matteo Pozza
Closes-Bug: #1927677
(cherry picked from commit 6fbd0b758d)
(cherry picked from commit 47dad4836a)
(cherry picked from commit 9588cdbfd4)
This commit is contained in:
Sean Mooney 2021-08-23 15:37:48 +01:00 committed by Elod Illes
parent cce1f9a0f0
commit 0997043f45
2 changed files with 35 additions and 6 deletions

View File

@ -297,14 +297,9 @@ class NovaProxyRequestHandler(websockify.ProxyRequestHandler):
if os.path.isdir(path): if os.path.isdir(path):
parts = urlparse.urlsplit(self.path) parts = urlparse.urlsplit(self.path)
if not parts.path.endswith('/'): if not parts.path.endswith('/'):
# redirect browser - doing basically what apache does
new_parts = (parts[0], parts[1], parts[2] + '/',
parts[3], parts[4])
new_url = urlparse.urlunsplit(new_parts)
# Browsers interpret "Location: //uri" as an absolute URI # Browsers interpret "Location: //uri" as an absolute URI
# like "http://URI" # like "http://URI"
if new_url.startswith('//'): if self.path.startswith('//'):
self.send_error(HTTPStatus.BAD_REQUEST, self.send_error(HTTPStatus.BAD_REQUEST,
"URI must not start with //") "URI must not start with //")
return None return None

View File

@ -659,6 +659,40 @@ class NovaProxyRequestHandlerTestCase(test.NoDBTestCase):
# Verify no redirect happens and instead a 400 Bad Request is returned. # Verify no redirect happens and instead a 400 Bad Request is returned.
self.assertIn('400 URI must not start with //', result[0].decode()) self.assertIn('400 URI must not start with //', result[0].decode())
def test_reject_open_redirect_3_slashes(self):
# This will test the behavior when an attempt is made to cause an open
# redirect. It should be rejected.
mock_req = mock.MagicMock()
mock_req.makefile().readline.side_effect = [
b'GET ///example.com/%2F.. HTTP/1.1\r\n',
b''
]
# Collect the response data to verify at the end. The
# SimpleHTTPRequestHandler writes the response data by calling the
# request socket sendall() method.
self.data = b''
def fake_sendall(data):
self.data += data
mock_req.sendall.side_effect = fake_sendall
client_addr = ('8.8.8.8', 54321)
mock_server = mock.MagicMock()
# This specifies that the server will be able to handle requests other
# than only websockets.
mock_server.only_upgrade = False
# Constructing a handler will process the mock_req request passed in.
websocketproxy.NovaProxyRequestHandler(
mock_req, client_addr, mock_server)
# Verify no redirect happens and instead a 400 Bad Request is returned.
self.data = self.data.decode()
self.assertIn('Error code: 400', self.data)
self.assertIn('Message: URI must not start with //', self.data)
@mock.patch('websockify.websocketproxy.select_ssl_version') @mock.patch('websockify.websocketproxy.select_ssl_version')
def test_ssl_min_version_is_not_set(self, mock_select_ssl): def test_ssl_min_version_is_not_set(self, mock_select_ssl):
websocketproxy.NovaWebSocketProxy() websocketproxy.NovaWebSocketProxy()