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+. Conflicts: nova/tests/unit/console/test_websocketproxy.py NOTE: conflict is due to I58b0382c86d4ef798572edb63d311e0e3e6937bb is missing in Victoria and Ie36401c782f023d1d5f2623732619105dc2cfa24 backport contained conflicts and methods order was swapped. Change-Id: I95f68be76330ff09e5eabb5ef8dd9a18f5547866 co-authored-by: Matteo Pozza Closes-Bug: #1927677 (cherry picked from commit6fbd0b758d
) (cherry picked from commit47dad4836a
)
This commit is contained in:
parent
1eceeebfb2
commit
9588cdbfd4
@ -290,14 +290,9 @@ class NovaProxyRequestHandler(websockify.ProxyRequestHandler):
|
||||
if os.path.isdir(path):
|
||||
parts = urlparse.urlsplit(self.path)
|
||||
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
|
||||
# like "http://URI"
|
||||
if new_url.startswith('//'):
|
||||
if self.path.startswith('//'):
|
||||
self.send_error(HTTPStatus.BAD_REQUEST,
|
||||
"URI must not start with //")
|
||||
return None
|
||||
|
@ -660,6 +660,40 @@ class NovaProxyRequestHandlerTestCase(test.NoDBTestCase):
|
||||
self.assertIn('Error code: 400', self.data)
|
||||
self.assertIn('Message: URI must not start with //', self.data)
|
||||
|
||||
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')
|
||||
def test_ssl_min_version_is_not_set(self, mock_select_ssl):
|
||||
websocketproxy.NovaWebSocketProxy()
|
||||
|
Loading…
Reference in New Issue
Block a user