[libsoup/websockets-fixes-2.66: 5/19] WebSockets: fail and close the connection in case of invalid payload length

[Claudio Saavedra <csaavedra src gnome org>]

commit ec89d5ea3be37815016a7a2c7dd1292fb0878804
Author: Carlos Garcia Campos <cgarcia igalia com>
Date:   Tue Jun 18 17:42:05 2019 +0200

    WebSockets: fail and close the connection in case of invalid payload length
    
    RFC 6455:
    
    The length of the "Payload data", in bytes: if 0-125, that is the
    payload length.  If 126, the following 2 bytes interpreted as a
    16-bit unsigned integer are the payload length.  If 127, the
    following 8 bytes interpreted as a 64-bit unsigned integer (the
    most significant bit MUST be 0) are the payload length.  Multibyte
    length quantities are expressed in network byte order.  Note that
    in all cases, the minimal number of bytes MUST be used to encode
    the length, for example, the length of a 124-byte-long string
    can't be encoded as the sequence 126, 0, 124.

 libsoup/soup-websocket-connection.c |  19 +++++++
 tests/websocket-test.c              | 103 ++++++++++++++++++++++++++++++++++++
 2 files changed, 122 insertions(+)
---
diff --git a/libsoup/soup-websocket-connection.c b/libsoup/soup-websocket-connection.c
index 7845fd0c..fb044656 100644
--- a/libsoup/soup-websocket-connection.c
+++ b/libsoup/soup-websocket-connection.c
@@ -839,13 +839,26 @@ process_frame (SoupWebsocketConnection *self)
 
        switch (header[1] & 0x7f) {
        case 126:
+               /* If 126, the following 2 bytes interpreted as a 16-bit
+                * unsigned integer are the payload length.
+                */
                at = 4;
                if (len < at)
                        return FALSE; /* need more data */
                payload_len = (((guint16)header[2] << 8) |
                               ((guint16)header[3] << 0));
+
+               /* The minimal number of bytes MUST be used to encode the length. */
+               if (payload_len <= 125) {
+                       protocol_error_and_close (self);
+                       return FALSE;
+               }
                break;
        case 127:
+               /* If 127, the following 8 bytes interpreted as a 64-bit
+                * unsigned integer (the most significant bit MUST be 0)
+                * are the payload length.
+                */
                at = 10;
                if (len < at)
                        return FALSE; /* need more data */
@@ -857,6 +870,12 @@ process_frame (SoupWebsocketConnection *self)
                               ((guint64)header[7] << 16) |
                               ((guint64)header[8] << 8) |
                               ((guint64)header[9] << 0));
+
+               /* The minimal number of bytes MUST be used to encode the length. */
+               if (payload_len <= G_MAXUINT16) {
+                       protocol_error_and_close (self);
+                       return FALSE;
+               }
                break;
        default:
                payload_len = header[1] & 0x7f;
diff --git a/tests/websocket-test.c b/tests/websocket-test.c
index 4015a96d..e03ca651 100644
--- a/tests/websocket-test.c
+++ b/tests/websocket-test.c
@@ -889,6 +889,99 @@ test_receive_fragmented (Test *test,
        WAIT_UNTIL (soup_websocket_connection_get_state (test->client) == SOUP_WEBSOCKET_STATE_CLOSED);
 }
 
+typedef struct {
+       Test *test;
+       const char *header;
+       GString *payload;
+} InvalidEncodeLengthTest;
+
+static gpointer
+send_invalid_encode_length_server_thread (gpointer user_data)
+{
+       InvalidEncodeLengthTest *test = user_data;
+       gsize header_size;
+       gsize written;
+       GError *error = NULL;
+
+       header_size = test->payload->len == 125 ? 6 : 10;
+       g_output_stream_write_all (g_io_stream_get_output_stream (test->test->raw_server),
+                                  test->header, header_size, &written, NULL, &error);
+       g_assert_no_error (error);
+       g_assert_cmpuint (written, ==, header_size);
+
+       g_output_stream_write_all (g_io_stream_get_output_stream (test->test->raw_server),
+                                  test->payload->str, test->payload->len, &written, NULL, &error);
+       g_assert_no_error (error);
+       g_assert_cmpuint (written, ==, test->payload->len);
+
+       g_io_stream_close (test->test->raw_server, NULL, &error);
+       g_assert_no_error (error);
+
+       return NULL;
+}
+
+static void
+test_receive_invalid_encode_length_16 (Test *test,
+                                      gconstpointer data)
+{
+       GThread *thread;
+       GBytes *received = NULL;
+       GError *error = NULL;
+       InvalidEncodeLengthTest context = { test, NULL };
+       guint i;
+
+       g_signal_connect (test->client, "error", G_CALLBACK (on_error_copy), &error);
+       g_signal_connect (test->client, "message", G_CALLBACK (on_binary_message), &received);
+
+       /* We use 126(~) as payload length with 125 extended length */
+       context.header = "\x82~\x00}";
+       context.payload = g_string_new (NULL);
+       for (i = 0; i < 125; i++)
+               g_string_append (context.payload, "X");
+       thread = g_thread_new ("invalid-encode-length-thread", send_invalid_encode_length_server_thread, 
&context);
+
+       WAIT_UNTIL (error != NULL || received != NULL);
+       g_assert_error (error, SOUP_WEBSOCKET_ERROR, SOUP_WEBSOCKET_CLOSE_PROTOCOL_ERROR);
+       g_clear_error (&error);
+       g_assert_null (received);
+
+       g_thread_join (thread);
+       g_string_free (context.payload, TRUE);
+
+       WAIT_UNTIL (soup_websocket_connection_get_state (test->client) == SOUP_WEBSOCKET_STATE_CLOSED);
+}
+
+static void
+test_receive_invalid_encode_length_64 (Test *test,
+                                      gconstpointer data)
+{
+       GThread *thread;
+       GBytes *received = NULL;
+       GError *error = NULL;
+       InvalidEncodeLengthTest context = { test, NULL };
+       guint i;
+
+       g_signal_connect (test->client, "error", G_CALLBACK (on_error_copy), &error);
+       g_signal_connect (test->client, "message", G_CALLBACK (on_binary_message), &received);
+
+       /* We use 127(\x7f) as payload length with 65535 extended length */
+       context.header = "\x82\x7f\x00\x00\x00\x00\x00\x00\xff\xff";
+       context.payload = g_string_new (NULL);
+       for (i = 0; i < 65535; i++)
+               g_string_append (context.payload, "X");
+       thread = g_thread_new ("invalid-encode-length-thread", send_invalid_encode_length_server_thread, 
&context);
+
+       WAIT_UNTIL (error != NULL || received != NULL);
+       g_assert_error (error, SOUP_WEBSOCKET_ERROR, SOUP_WEBSOCKET_CLOSE_PROTOCOL_ERROR);
+       g_clear_error (&error);
+       g_assert_null (received);
+
+        g_thread_join (thread);
+       g_string_free (context.payload, TRUE);
+
+       WAIT_UNTIL (soup_websocket_connection_get_state (test->client) == SOUP_WEBSOCKET_STATE_CLOSED);
+}
+
 static void
 test_client_context_got_server_connection (SoupServer              *server,
                                           SoupWebsocketConnection *connection,
@@ -1061,6 +1154,16 @@ main (int argc,
                    test_receive_fragmented,
                    teardown_direct_connection);
 
+       g_test_add ("/websocket/direct/receive-invalid-encode-length-16", Test, NULL,
+                   setup_half_direct_connection,
+                   test_receive_invalid_encode_length_16,
+                   teardown_direct_connection);
+
+       g_test_add ("/websocket/direct/receive-invalid-encode-length-64", Test, NULL,
+                   setup_half_direct_connection,
+                   test_receive_invalid_encode_length_64,
+                   teardown_direct_connection);
+
        if (g_test_slow ()) {
                g_test_add ("/websocket/direct/close-after-timeout", Test, NULL,
                            setup_half_direct_connection,