From d86d346450476e7f4b7d5af79d6808d13b29e292 Mon Sep 17 00:00:00 2001 From: Carlos Garcia Campos Date: Wed, 28 Sep 2022 12:44:20 +0200 Subject: 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, -- cgit v1.2.1