Skip to content

Commit

Permalink
websocket: Limit maximum uncompressed frame length to 8MiB
Browse files Browse the repository at this point in the history
This fixes a memory exhaustion DOS attack vector.

References: GHSA-9p9m-jm8w-94p2
GHSA-9p9m-jm8w-94p2
  • Loading branch information
onnokort authored and temoto committed May 5, 2021
1 parent b0be94e commit 1412f5e
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 7 deletions.
34 changes: 28 additions & 6 deletions eventlet/websocket.py
Expand Up @@ -38,6 +38,7 @@
break

ACCEPTABLE_CLIENT_ERRORS = set((errno.ECONNRESET, errno.EPIPE))
DEFAULT_MAX_FRAME_LENGTH = 8 << 20

__all__ = ["WebSocketWSGI", "WebSocket"]
PROTOCOL_GUID = b'258EAFA5-E914-47DA-95CA-C5AB0DC85B11'
Expand Down Expand Up @@ -75,14 +76,20 @@ def my_handler(ws):
:class:`WebSocket`. To close the socket, simply return from the
function. Note that the server will log the websocket request at
the time of closure.
An optional argument max_frame_length can be given, which will set the
maximum incoming *uncompressed* payload length of a frame. By default, this
is set to 8MiB. Note that excessive values here might create a DOS attack
vector.
"""

def __init__(self, handler):
def __init__(self, handler, max_frame_length=DEFAULT_MAX_FRAME_LENGTH):
self.handler = handler
self.protocol_version = None
self.support_legacy_versions = True
self.supported_protocols = []
self.origin_checker = None
self.max_frame_length = max_frame_length

@classmethod
def configured(cls,
Expand Down Expand Up @@ -324,7 +331,8 @@ def _handle_hybi_request(self, environ):
sock.sendall(b'\r\n'.join(handshake_reply) + b'\r\n\r\n')
return RFC6455WebSocket(sock, environ, self.protocol_version,
protocol=negotiated_protocol,
extensions=parsed_extensions)
extensions=parsed_extensions,
max_frame_length=self.max_frame_length)

def _extract_number(self, value):
"""
Expand Down Expand Up @@ -503,7 +511,8 @@ class ProtocolError(ValueError):


class RFC6455WebSocket(WebSocket):
def __init__(self, sock, environ, version=13, protocol=None, client=False, extensions=None):
def __init__(self, sock, environ, version=13, protocol=None, client=False, extensions=None,
max_frame_length=DEFAULT_MAX_FRAME_LENGTH):
super(RFC6455WebSocket, self).__init__(sock, environ, version)
self.iterator = self._iter_frames()
self.client = client
Expand All @@ -512,6 +521,8 @@ def __init__(self, sock, environ, version=13, protocol=None, client=False, exten

self._deflate_enc = None
self._deflate_dec = None
self.max_frame_length = max_frame_length
self._remote_close_data = None

class UTF8Decoder(object):
def __init__(self):
Expand Down Expand Up @@ -583,12 +594,13 @@ def _get_bytes(self, numbytes):
return data

class Message(object):
def __init__(self, opcode, decoder=None, decompressor=None):
def __init__(self, opcode, max_frame_length, decoder=None, decompressor=None):
self.decoder = decoder
self.data = []
self.finished = False
self.opcode = opcode
self.decompressor = decompressor
self.max_frame_length = max_frame_length

def push(self, data, final=False):
self.finished = final
Expand All @@ -597,7 +609,12 @@ def push(self, data, final=False):
def getvalue(self):
data = b"".join(self.data)
if not self.opcode & 8 and self.decompressor:
data = self.decompressor.decompress(data + b'\x00\x00\xff\xff')
data = self.decompressor.decompress(data + b"\x00\x00\xff\xff", self.max_frame_length)
if self.decompressor.unconsumed_tail:
raise FailedConnectionError(
1009,
"Incoming compressed frame exceeds length limit of {} bytes.".format(self.max_frame_length))

if self.decoder:
data = self.decoder.decode(data, self.finished)
return data
Expand All @@ -611,6 +628,7 @@ def _apply_mask(data, mask, length=None, offset=0):

def _handle_control_frame(self, opcode, data):
if opcode == 8: # connection close
self._remote_close_data = data
if not data:
status = 1000
elif len(data) > 1:
Expand Down Expand Up @@ -710,13 +728,17 @@ def _recv_frame(self, message=None):
length = struct.unpack('!H', recv(2))[0]
elif length == 127:
length = struct.unpack('!Q', recv(8))[0]

if length > self.max_frame_length:
raise FailedConnectionError(1009, "Incoming frame of {} bytes is above length limit of {} bytes.".format(
length, self.max_frame_length))
if masked:
mask = struct.unpack('!BBBB', recv(4))
received = 0
if not message or opcode & 8:
decoder = self.UTF8Decoder() if opcode == 1 else None
decompressor = self._get_permessage_deflate_dec(rsv1)
message = self.Message(opcode, decoder=decoder, decompressor=decompressor)
message = self.Message(opcode, self.max_frame_length, decoder=decoder, decompressor=decompressor)
if not length:
message.push(b'', final=finished)
else:
Expand Down
59 changes: 58 additions & 1 deletion tests/websocket_new_test.py
Expand Up @@ -30,7 +30,12 @@ def handle(ws):
else:
ws.close()

wsapp = websocket.WebSocketWSGI(handle)

# Set a lower limit of DEFAULT_MAX_FRAME_LENGTH for testing, as
# sending an 8MiB frame over the loopback interface can trigger a
# timeout.
TEST_MAX_FRAME_LENGTH = 50000
wsapp = websocket.WebSocketWSGI(handle, max_frame_length=TEST_MAX_FRAME_LENGTH)


class TestWebSocket(tests.wsgi_test._TestBase):
Expand Down Expand Up @@ -534,3 +539,55 @@ def test_compressed_send_recv_both_no_context_13(self):

ws.close()
eventlet.sleep(0.01)

def test_large_frame_size_compressed_13(self):
# Test fix for GHSA-9p9m-jm8w-94p2
extensions_string = 'permessage-deflate'
extensions = {'permessage-deflate': {
'client_no_context_takeover': False,
'server_no_context_takeover': False}}

sock = eventlet.connect(self.server_addr)
sock.sendall(six.b(self.connect % extensions_string))
sock.recv(1024)
ws = websocket.RFC6455WebSocket(sock, {}, client=True, extensions=extensions)

should_still_fit = b"x" * TEST_MAX_FRAME_LENGTH
one_too_much = should_still_fit + b"x"

# send just fitting frame twice to make sure they are fine independently
ws.send(should_still_fit)
assert ws.wait() == should_still_fit
ws.send(should_still_fit)
assert ws.wait() == should_still_fit
ws.send(one_too_much)

res = ws.wait()
assert res is None # socket closed
# TODO: The websocket currently sents compressed control frames, which contradicts RFC7692.
# Renable the following assert after that has been fixed.
# assert ws._remote_close_data == b"\x03\xf1Incoming compressed frame is above length limit."
eventlet.sleep(0.01)

def test_large_frame_size_uncompressed_13(self):
# Test fix for GHSA-9p9m-jm8w-94p2
sock = eventlet.connect(self.server_addr)
sock.sendall(six.b(self.connect))
sock.recv(1024)
ws = websocket.RFC6455WebSocket(sock, {}, client=True)

should_still_fit = b"x" * TEST_MAX_FRAME_LENGTH
one_too_much = should_still_fit + b"x"

# send just fitting frame twice to make sure they are fine independently
ws.send(should_still_fit)
assert ws.wait() == should_still_fit
ws.send(should_still_fit)
assert ws.wait() == should_still_fit
ws.send(one_too_much)

res = ws.wait()
assert res is None # socket closed
# close code should be available now
assert ws._remote_close_data == b"\x03\xf1Incoming frame of 50001 bytes is above length limit of 50000 bytes."
eventlet.sleep(0.01)

3 comments on commit 1412f5e

@carnil
Copy link

@carnil carnil commented on 1412f5e May 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@onnokort, @temoto: GHSA-9p9m-jm8w-94p2 claims that affected versions are >= 0.10.0, but the per message-defalte extension or compression extension
was added by b7d2a25 which would be later than 0.10.0. Is this assessment correct? Or ist the issue still present before starting from versions 0.10.0?

@onnokort
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this assessment correct? Or ist the issue still present before starting from versions 0.10.0?

There were two issues that are fixed by this commit. One is the compression data explosion and the other is that there was no length check being performed on the websocket frames, which can be up to 2^64-1 in size (payload) IIRC.

This latter issue might exist in previous versions that have no compression support, but I haven't checked. It would not be as easily exploitable as the compression problem, however it would still allow a remote host to overflow the server's memory by sending a huge websocket frame.

@carnil
Copy link

@carnil carnil commented on 1412f5e May 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@onnokort thanks for the quick confirmation!

Please sign in to comment.