[libsoup/carlosgc/ws-invalid-close-codes] websocket: handle more invalid close codes

[Carlos Garcia Campos <carlosgc src gnome org>]

commit d86d346450476e7f4b7d5af79d6808d13b29e292
Author: Carlos Garcia Campos <cgarcia igalia com>
Date:   Wed Sep 28 12:44:20 2022 +0200

    websocket: handle more invalid close codes

 libsoup/websocket/soup-websocket-connection.c | 14 ++++++++++++-
 tests/websocket-test.c                        | 30 +++++++++++++++++++++++----
 2 files changed, 39 insertions(+), 5 deletions(-)
---
diff --git a/libsoup/websocket/soup-websocket-connection.c b/libsoup/websocket/soup-websocket-connection.c
index fcd4fd36..f1aaba24 100644
--- a/libsoup/websocket/soup-websocket-connection.c
+++ b/libsoup/websocket/soup-websocket-connection.c
@@ -719,7 +719,7 @@ close_connection (SoupWebsocketConnection *self,
                code = 0;
                break;
        default:
-               if (code < 3000) {
+               if (code < 3000 || code >= 5000) {
                        g_debug ("Wrong closing code %d received", code);
                        protocol_error_and_close (self);
                        return;
@@ -766,6 +766,18 @@ receive_close (SoupWebsocketConnection *self,
                break;
        }
 
+        /* 1005, 1006 and 1015 are reserved values and MUST NOT be set as a status code in a Close control 
frame by an endpoint */
+        switch (priv->peer_close_code) {
+        case SOUP_WEBSOCKET_CLOSE_NO_STATUS:
+        case SOUP_WEBSOCKET_CLOSE_ABNORMAL:
+        case SOUP_WEBSOCKET_CLOSE_TLS_HANDSHAKE:
+                g_debug ("received a broken close frame containing reserved status code %u", 
priv->peer_close_code);
+                protocol_error_and_close (self);
+                return;
+        default:
+                break;
+        }
+
        if (len > 2) {
                data += 2;
                len -= 2;
diff --git a/tests/websocket-test.c b/tests/websocket-test.c
index 68be0dc2..e351443a 100644
--- a/tests/websocket-test.c
+++ b/tests/websocket-test.c
@@ -907,6 +907,11 @@ test_protocol_client_any_soup (Test *test,
        g_assert_cmpstr (soup_message_headers_get_one (soup_message_get_response_headers (test->msg), 
"Sec-WebSocket-Protocol"), ==, NULL);
 }
 
+typedef enum {
+        CLOSE_TEST_FLAG_SERVER = 1 << 0,
+        CLOSE_TEST_FLAG_CLIENT = 1 << 1
+} CloseTestFlags;
+
 static const struct {
        gushort code;
        const char *reason;
@@ -914,11 +919,16 @@ static const struct {
        const char *expected_sender_reason;
        gushort expected_receiver_code;
        const char *expected_receiver_reason;
+        CloseTestFlags flags;
 } close_clean_tests[] = {
-       { SOUP_WEBSOCKET_CLOSE_NORMAL, "NORMAL", SOUP_WEBSOCKET_CLOSE_NORMAL, "NORMAL", 
SOUP_WEBSOCKET_CLOSE_NORMAL, "NORMAL" },
-       { SOUP_WEBSOCKET_CLOSE_GOING_AWAY, "GOING_AWAY", SOUP_WEBSOCKET_CLOSE_GOING_AWAY, "GOING_AWAY", 
SOUP_WEBSOCKET_CLOSE_GOING_AWAY, "GOING_AWAY" },
-       { SOUP_WEBSOCKET_CLOSE_NORMAL, NULL, SOUP_WEBSOCKET_CLOSE_NORMAL, NULL, SOUP_WEBSOCKET_CLOSE_NORMAL, 
NULL },
-       { SOUP_WEBSOCKET_CLOSE_NO_STATUS, NULL, SOUP_WEBSOCKET_CLOSE_NORMAL, NULL, 
SOUP_WEBSOCKET_CLOSE_NO_STATUS, NULL },
+       { SOUP_WEBSOCKET_CLOSE_NORMAL, "NORMAL", SOUP_WEBSOCKET_CLOSE_NORMAL, "NORMAL", 
SOUP_WEBSOCKET_CLOSE_NORMAL, "NORMAL", CLOSE_TEST_FLAG_SERVER | CLOSE_TEST_FLAG_CLIENT },
+       { SOUP_WEBSOCKET_CLOSE_GOING_AWAY, "GOING_AWAY", SOUP_WEBSOCKET_CLOSE_GOING_AWAY, "GOING_AWAY", 
SOUP_WEBSOCKET_CLOSE_GOING_AWAY, "GOING_AWAY", CLOSE_TEST_FLAG_SERVER | CLOSE_TEST_FLAG_CLIENT },
+       { SOUP_WEBSOCKET_CLOSE_NORMAL, NULL, SOUP_WEBSOCKET_CLOSE_NORMAL, NULL, SOUP_WEBSOCKET_CLOSE_NORMAL, 
NULL, CLOSE_TEST_FLAG_SERVER | CLOSE_TEST_FLAG_CLIENT },
+       { SOUP_WEBSOCKET_CLOSE_NO_STATUS, NULL, SOUP_WEBSOCKET_CLOSE_NORMAL, NULL, 
SOUP_WEBSOCKET_CLOSE_NO_STATUS, NULL, CLOSE_TEST_FLAG_SERVER | CLOSE_TEST_FLAG_CLIENT },
+        { 2999, NULL, SOUP_WEBSOCKET_CLOSE_PROTOCOL_ERROR, NULL, SOUP_WEBSOCKET_CLOSE_PROTOCOL_ERROR, NULL, 
CLOSE_TEST_FLAG_CLIENT },
+        { 2999, NULL, 0, NULL, SOUP_WEBSOCKET_CLOSE_PROTOCOL_ERROR, NULL, CLOSE_TEST_FLAG_SERVER },
+        { 5000, NULL, SOUP_WEBSOCKET_CLOSE_PROTOCOL_ERROR, NULL, SOUP_WEBSOCKET_CLOSE_PROTOCOL_ERROR, NULL, 
CLOSE_TEST_FLAG_CLIENT },
+        { 5000, NULL, 0, NULL, SOUP_WEBSOCKET_CLOSE_PROTOCOL_ERROR, NULL, CLOSE_TEST_FLAG_SERVER },
 };
 
 static void
@@ -958,6 +968,9 @@ test_close_clean_client_soup (Test *test,
        guint i;
 
        for (i = 0; i < G_N_ELEMENTS (close_clean_tests); i++) {
+                if (!(close_clean_tests[i].flags & CLOSE_TEST_FLAG_CLIENT))
+                        continue;
+
                setup_soup_connection (test, data);
 
                do_close_clean_client (test,
@@ -979,6 +992,9 @@ test_close_clean_client_direct (Test *test,
        guint i;
 
        for (i = 0; i < G_N_ELEMENTS (close_clean_tests); i++) {
+                if (!(close_clean_tests[i].flags & CLOSE_TEST_FLAG_CLIENT))
+                        continue;
+
                setup_direct_connection (test, data);
 
                do_close_clean_client (test,
@@ -1030,6 +1046,9 @@ test_close_clean_server_soup (Test *test,
        guint i;
 
        for (i = 0; i < G_N_ELEMENTS (close_clean_tests); i++) {
+                if (!(close_clean_tests[i].flags & CLOSE_TEST_FLAG_SERVER))
+                        continue;
+
                setup_direct_connection (test, data);
 
                do_close_clean_server (test,
@@ -1051,6 +1070,9 @@ test_close_clean_server_direct (Test *test,
        guint i;
 
        for (i = 0; i < G_N_ELEMENTS (close_clean_tests); i++) {
+                if (!(close_clean_tests[i].flags & CLOSE_TEST_FLAG_SERVER))
+                        continue;
+
                setup_direct_connection (test, data);
 
                do_close_clean_server (test,