uNetworking / uWebSockets

Simple, secure & standards compliant web server for the most demanding of applications
Apache License 2.0
17.43k stars 1.76k forks source link

maxPayloadLength large value - connection not closed with some clients #979

Closed slavslavov closed 4 years ago

slavslavov commented 4 years ago

I am trying several clients and noticed with one of them I was able to send/receive data larger than 1MB while maxPayloadLength was set to 1MB. The client is https://github.com/machinezone/IXWebSocket

Way to reproduce the issue:

Tried a Python client and it works as expected, the connection is closed when data is larger than 1MB. Here is the code (using this client: https://pypi.org/project/websocket_client/):

from websocket import create_connection

import random
import string

def randomString(stringLength=10):
    """Generate a random string of fixed length """
    letters = string.ascii_lowercase
    return ''.join(random.choice(letters) for i in range(stringLength))

st = randomString(1024 * 1024 + 1)

ws = create_connection("ws://localhost:9001/")
print("Sending")
ws.send(st)
print("Sent")
print("Receiving...")
result =  ws.recv()
if st == result:
    print("Received ")
else:
    print("Error")
ws.close()
ghost commented 4 years ago

Disable compression. It is not very strict with compression

slavslavov commented 4 years ago

I tried it now but I am still able to reproduce the issue. The IXWebSocket executable is about 32MB so I was able to send it and receive it successfully. Looks like it is sending the data in chunks:

[slav@localhost ws]$ ls -l
-rwxrwxr-x 1 slav slav 32467208 Dec 29 22:56 ws

[slav@localhost ws]$ ./ws send ws://localhost:9001/ ws
[2019-12-29 23:28:13.580] [info] ws_send: Connecting to url: ws://localhost:9001/
[2019-12-29 23:28:13.580] [info] ws_send: Connecting...
[2019-12-29 23:28:13.582] [info] ws_send: connected
[2019-12-29 23:28:13.582] [info] Uri: /
[2019-12-29 23:28:13.582] [info] Headers:
[2019-12-29 23:28:13.582] [info] ws_send: Sending...
[2019-12-29 23:28:13.582] [info] Connection: Upgrade
[2019-12-29 23:28:13.583] [info] Sec-WebSocket-Accept: AReQ8uccWKM1JBGOE3hT6xxG5HE=
[2019-12-29 23:28:13.583] [info] Upgrade: websocket
[2019-12-29 23:28:13.583] [info] uWebSockets: v0.17
[2019-12-29 23:28:13.654] [info] load file from disk completed in 71
[2019-12-29 23:28:14.350] [info] ws_send: Step 0 out of 990
[2019-12-29 23:28:14.351] [info] ws_send: Step 1 out of 990
[2019-12-29 23:28:14.351] [info] ws_send: Step 2 out of 990
[2019-12-29 23:28:14.352] [info] ws_send: Step 3 out of 990
...
[2019-12-29 23:28:15.154] [info] ws_send: Step 988 out of 990
[2019-12-29 23:28:15.155] [info] ws_send: Step 989 out of 990
[2019-12-29 23:28:15.156] [info] ws_send: 0 bytes left to be sent
[2019-12-29 23:28:15.167] [info] Sending file through websocket completed in 1237
[2019-12-29 23:28:15.167] [info] ws_send: Send transfer rate: 25 MB/s
[2019-12-29 23:28:15.169] [info] ws_send: Waiting for ack...
[2019-12-29 23:28:15.523] [info] ws_send: received message (32467309 bytes)
[2019-12-29 23:28:15.523] [info] ws_send: Done !
[2019-12-29 23:28:16.126] [info] ws_send: connection closed: code 1000 reason Normal closure
ghost commented 4 years ago
Hello Alex,

I guess you closed #979 to minimise the noise because it is a very serious vulnerability. I decided to do some more tests and debugging and will present you the results here.

I created a VM with 512MB RAM and run there EchoServer with maxPayloadLength set to 1MB. From two terminals on my PC, with the IXWebSockect client I then send a 450MB file to the server. I watch the load on the server with htop. At some point All available RAM and swap on the server is used and one of the clients still trying to resend the remainder of the file. Opening third console with IXWebSockect client and trying to send a smaller file can't connect at all.

With the default setting of 16K, if an attacker sends fragments of 8K for example he will be able to compromise the whole system.

The good thing is EchoServer didn't crash. Once I terminated the clients I could send smaller files.

I did some debugging and noticed maxPayloadLength can do the job for checking the payload size per frame. If the message is fragmented (as IXWebSockect fragments to 32k chunks) there is no way to prevent the client flooding the server.

One easy fix is to call refusePayloadLengthjust before webSocketData->fragmentBuffer.append(data, length) in handleFragment as shown:

                /* Allocate fragment buffer up front first time */
                if (!webSocketData->fragmentBuffer.length()) {
                    webSocketData->fragmentBuffer.reserve(length + remainingBytes);
                }
                if (refusePayloadLength(length + webSocketData->fragmentBuffer.length(), webSocketState, s)) {
                    forceClose(webSocketState, s);
                    return true;
                }
                webSocketData->fragmentBuffer.append(data, length);

But I am sure you'll find a better way of doing it.
It might be better to introduce a new configuration setting - for example maxMessageloadLength for this.
It will be great if you could gracefully close the connection with code 1009 as described in RFC 6455.

Kind regards,
Slav

I guess you closed #979 to minimise the noise because it is a very serious vulnerability

Please have a look in fuzzing folder.

I close a lot of issues daily. Was there something I misunderstood?

ghost commented 4 years ago

I see what you mean now - I have applied that fix and will probably add a check to the fuzz targets. Thanks for making it clear.