Wrong data_ready_len value being passed to websocket callback causing segfault

edufschmidt commented 8 years ago

Hi there!

I have recently faced an issue when sending messages larger than 125 bytes to a websocket server written using onion. I noticed that when the data received is shorter than 125 bytes, everything works fine. However, as soon as a payload of 126 bytes or more is received, the server crashes with a segfault. I logged the third argument in the websocket callback (data_ready_len), and it is seems to assume an incorrect value if the received payload is larger than 125 bytes. It's interesting that a payload length of 126, for instance, caused the data_ready_len value to become 0x7E00, instead of 0x7E, as expected. When the payload size is of 127 bytes (0x7F), the data_ready_len value is of 0x7F00. I believe this could also be the cause of the segfault, and might indicate an endianess issue, or some incorrect masking somewhere within Onion. Below are my server and client (js) code:

Server:

onion_connection_status websocket_handler (void *data, onion_websocket *ws,
                                           ssize_t data_ready_len) {
    char buf[256];
    bzero (buf, sizeof (buf));
    int len = onion_websocket_read (ws, buf, 1);
    fprintf (stderr, "read %d: %s (data_ready = %x / %ld)\n", len, buf,
             data_ready_len, data_ready_len);

    return OCS_NEED_MORE_DATA;
}

onion_connection_status handler_1 (void *data, onion_request *req,
                                   onion_response *res) {
    onion_websocket *ws = onion_websocket_new (req, res);

    if (ws) {
        onion_websocket_printf (ws, "Hello from server.");
        onion_websocket_set_callback (ws, websocket_handler);
        return OCS_WEBSOCKET;
    }
    return OCS_PROCESSED;
}

int main () {
    onion *o = onion_new (O_ONE);
    onion_set_port (o, "9999");
    onion_url *urls = onion_root_url (o);
    onion_url_add (urls, "", handler_1);
    onion_listen (o);
    onion_free (o);
    return 0;
}

Client:

<!DOCTYPE HTML>
<html>
<head>
    <script type="text/javascript">
        var ws = {};

        function sendLongMessage() {
            // 126 characters message
            var msg = 'abcdefghijklmnopqrstuvxzABCDEFGHIJKLMNOPQRSTUVXZ#!' +
                    'abcdefghijklmnopqrstuvxzABCDEFGHIJKLMNOPQRSTUVXZ#!' +
                    'abcdefghijklmnopqrstuvxz#!'
            ws.send(msg);
        }

        function sendShortMessage() {
            // 125 characters message
            var msg = 'abcdefghijklmnopqrstuvxzABCDEFGHIJKLMNOPQRSTUVXZ#!' +
                    'abcdefghijklmnopqrstuvxzABCDEFGHIJKLMNOPQRSTUVXZ#!' +
                    'abcdefghijklmnopqrstuvxz#'
            ws.send(msg);
        }

        function disconnect() {
            ws.close();
        }

        function connect() {
            ws = new WebSocket("ws://127.0.0.1:9999/");

            ws.onopen = function (evt) {
                console.log(evt);
            };

            ws.onmessage = function (evt) {
                console.log(evt);
            };

            ws.onclose = function (evt) {
                console.log(evt);
            };
        }
    </script>
</head>

<body>
<div>
    <button onclick="connect()">Connect</button>
    <button onclick="disconnect()">Disconnect</button>
    <button onclick="sendLongMessage()">Send long message</button>
    <button onclick="sendShortMessage()">Send short message</button>
</div>
</body>
</html>

PS.: I performed all tests in a 64 bit PC running Linux (Linux 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux)

Please let me know if I can help with more information, or if you think the problem is caused by some mistake on my end.

davidmoreno commented 8 years ago

Thanks a lot for the report and the code example. With it, it was quite easy to fix the problems.

edufschmidt commented 8 years ago

Thank you for the quick reply and for this awesome library!

edufschmidt commented 8 years ago

Hi David, I'm forced to ask you to reopen this issue. I've conducted a few more tests and the problem persists, even though the minimum message length for it to happen is a bit larger now (don't know the exact length yet).

davidmoreno commented 8 years ago

I will try to do more tests. If you got more information please post it here.

fdev1 commented 6 years ago

I'm having the same problem. When receiving about 350 bytes data_ready_length reports over 30K bytes. onion_websocket_read() returns the right number of bytes but the callback fires again but this time onion_websocket_read() returns EAGAIN and segfaults soon after the callback returns.

davidmoreno / onion

Wrong data_ready_len value being passed to websocket callback causing segfault #207