libsoup3/CVE-2026-4271.patch
2026-05-11 12:42:23 -04:00

362 lines
13 KiB
Diff

From 2da78194fff0775a6cec51799a4aedf5aee7618a Mon Sep 17 00:00:00 2001
From: Carlos Garcia Campos <cgarcia@igalia.com>
Date: Mon, 16 Feb 2026 12:09:08 +0100
Subject: [PATCH] server: protect message io while reading and writing
Ensure the nghttp2 session is not destroyed while being used.
Closes #496
---
.../http2/soup-server-message-io-http2.c | 117 +++++++++++++-----
tests/http2-test.c | 54 ++++++++
2 files changed, 141 insertions(+), 30 deletions(-)
diff --git a/libsoup/server/http2/soup-server-message-io-http2.c b/libsoup/server/http2/soup-server-message-io-http2.c
index 913afb46..6f8d1bb6 100644
--- a/libsoup/server/http2/soup-server-message-io-http2.c
+++ b/libsoup/server/http2/soup-server-message-io-http2.c
@@ -69,6 +69,8 @@ typedef struct {
GHashTable *messages;
guint in_callback;
+ guint protected;
+ gboolean destroyed;
} SoupServerMessageIOHTTP2;
static void soup_server_message_io_http2_send_response (SoupServerMessageIOHTTP2 *io,
@@ -146,6 +148,8 @@ soup_server_message_io_http2_destroy (SoupServerMessageIO *iface)
{
SoupServerMessageIOHTTP2 *io = (SoupServerMessageIOHTTP2 *)iface;
+ io->destroyed = TRUE;
+
if (io->read_source) {
g_source_destroy (io->read_source);
g_source_unref (io->read_source);
@@ -160,10 +164,14 @@ soup_server_message_io_http2_destroy (SoupServerMessageIO *iface)
}
g_clear_object (&io->iostream);
- g_clear_pointer (&io->session, nghttp2_session_del);
- g_clear_pointer (&io->messages, g_hash_table_unref);
+ io->istream = NULL;
+ io->ostream = NULL;
- g_free (io);
+ if (io->protected == 0) {
+ g_clear_pointer (&io->session, nghttp2_session_del);
+ g_clear_pointer (&io->messages, g_hash_table_unref);
+ g_free (io);
+ }
}
static void
@@ -321,7 +329,33 @@ static const SoupServerMessageIOFuncs io_funcs = {
soup_server_message_io_http2_is_paused
};
+static void
+soup_server_message_io_http2_protect (SoupServerMessageIOHTTP2 *io)
+{
+ io->protected++;
+ g_object_ref (io->conn);
+}
+
static gboolean
+soup_server_message_io_http2_unprotect (SoupServerMessageIOHTTP2 *io)
+{
+ g_object_unref (io->conn);
+
+ if (--io->protected > 0)
+ return FALSE;
+
+ if (io->destroyed) {
+ g_clear_pointer (&io->session, nghttp2_session_del);
+ g_clear_pointer (&io->messages, g_hash_table_unref);
+ g_free (io);
+
+ return TRUE;
+ }
+
+ return FALSE;
+}
+
+static void
io_write (SoupServerMessageIOHTTP2 *io,
GError **error)
{
@@ -336,51 +370,57 @@ io_write (SoupServerMessageIOHTTP2 *io,
if (io->write_buffer_size == 0) {
/* Done */
io->write_buffer = NULL;
- return TRUE;
+ return;
}
}
+ if (!io->ostream)
+ return;
+
gssize ret = g_pollable_stream_write (io->ostream,
io->write_buffer + io->written_bytes,
io->write_buffer_size - io->written_bytes,
FALSE, NULL, error);
- if (ret < 0)
- return FALSE;
-
- io->written_bytes += ret;
- return TRUE;
+ if (ret > 0)
+ io->written_bytes += ret;
}
static gboolean
io_write_ready (GObject *stream,
SoupServerMessageIOHTTP2 *io)
{
- SoupServerConnection *conn = io->conn;
GError *error = NULL;
- g_object_ref (conn);
+ soup_server_message_io_http2_protect (io);
+
+ while (!error) {
+ if (io->destroyed)
+ break;
+
+ if (!nghttp2_session_want_write (io->session))
+ break;
- while (!error && soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io && nghttp2_session_want_write (io->session))
io_write (io, &error);
+ }
if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
g_error_free (error);
- g_object_unref (conn);
+ soup_server_message_io_http2_unprotect (io);
return G_SOURCE_CONTINUE;
}
- if (soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io) {
+ if (!io->destroyed) {
if (error)
h2_debug (io, NULL, "[SESSION] IO error: %s", error->message);
g_clear_pointer (&io->write_source, g_source_unref);
if (error || (!nghttp2_session_want_read (io->session) && !nghttp2_session_want_write (io->session)))
- soup_server_connection_disconnect (conn);
+ soup_server_connection_disconnect (io->conn);
}
g_clear_error (&error);
- g_object_unref (conn);
+ soup_server_message_io_http2_unprotect (io);
return G_SOURCE_REMOVE;
}
@@ -390,13 +430,12 @@ static gboolean io_write_idle_cb (SoupServerMessageIOHTTP2* io);
static void
io_try_write (SoupServerMessageIOHTTP2 *io)
{
- SoupServerConnection *conn = io->conn;
GError *error = NULL;
if (io->write_source)
return;
- if (io->in_callback && soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io) {
+ if (io->in_callback && !io->destroyed) {
if (!nghttp2_session_want_write (io->session))
return;
@@ -416,12 +455,19 @@ io_try_write (SoupServerMessageIOHTTP2 *io)
g_clear_pointer (&io->write_idle_source, g_source_unref);
}
- g_object_ref (conn);
+ soup_server_message_io_http2_protect (io);
+
+ while (!error) {
+ if (io->destroyed)
+ break;
+
+ if (!nghttp2_session_want_write (io->session))
+ break;
- while (!error && soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io && !io->in_callback && nghttp2_session_want_write (io->session))
io_write (io, &error);
+ }
- if (soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io) {
+ if (!io->destroyed) {
if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
g_clear_error (&error);
io->write_source = g_pollable_output_stream_create_source (G_POLLABLE_OUTPUT_STREAM (io->ostream), NULL);
@@ -434,11 +480,11 @@ io_try_write (SoupServerMessageIOHTTP2 *io)
h2_debug (io, NULL, "[SESSION] IO error: %s", error->message);
if (error || (!nghttp2_session_want_read (io->session) && !nghttp2_session_want_write (io->session)))
- soup_server_connection_disconnect (conn);
+ soup_server_connection_disconnect (io->conn);
}
g_clear_error (&error);
- g_object_unref (conn);
+ soup_server_message_io_http2_unprotect (io);
}
static gboolean
@@ -481,31 +527,37 @@ static gboolean
io_read_ready (GObject *stream,
SoupServerMessageIOHTTP2 *io)
{
- SoupServerConnection *conn = io->conn;
gboolean progress = TRUE;
GError *error = NULL;
- g_object_ref (conn);
+ soup_server_message_io_http2_protect (io);
+
+ while (progress) {
+ if (io->destroyed)
+ break;
+
+ if (!nghttp2_session_want_read (io->session))
+ break;
- while (progress && soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io && nghttp2_session_want_read (io->session))
progress = io_read (io, &error);
+ }
if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
g_error_free (error);
- g_object_unref (conn);
+ soup_server_message_io_http2_unprotect (io);
return G_SOURCE_CONTINUE;
}
- if (soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io) {
+ if (!io->destroyed) {
if (error)
h2_debug (io, NULL, "[SESSION] IO error: %s", error->message);
if (error || (!nghttp2_session_want_read (io->session) && !nghttp2_session_want_write (io->session)))
- soup_server_connection_disconnect (conn);
+ soup_server_connection_disconnect (io->conn);
}
g_clear_error (&error);
- g_object_unref (conn);
+ soup_server_message_io_http2_unprotect (io);
return G_SOURCE_REMOVE;
}
@@ -931,5 +983,10 @@ soup_server_message_io_http2_new (SoupServerConnection *conn,
nghttp2_submit_settings (io->session, NGHTTP2_FLAG_NONE, settings, G_N_ELEMENTS (settings));
io_try_write (io);
+#ifdef __clang_analyzer__
+ // Suppress false positive about io being destroyed here, since at this point we have only
+ // send the initial settings and not callback is called.
+ [[clang::suppress]]
+#endif
return (SoupServerMessageIO *)io;
}
diff --git a/tests/http2-test.c b/tests/http2-test.c
index df86d9bb..7973f481 100644
--- a/tests/http2-test.c
+++ b/tests/http2-test.c
@@ -1265,6 +1265,40 @@ do_broken_pseudo_header_test (Test *test, gconstpointer data)
g_uri_unref (uri);
}
+static void
+disconnect_on_got_headers (SoupServerMessage *msg, gpointer user_data)
+{
+ GUri *uri;
+ SoupServerConnection *conn;
+
+ uri = soup_server_message_get_uri (msg);
+ if (!g_str_equal (g_uri_get_path (uri), "/close-on-got-headers"))
+ return;
+
+ conn = soup_server_message_get_connection (msg);
+ soup_server_connection_disconnect (conn);
+}
+
+static void
+do_server_disconnect_on_got_headers_test (Test *test, gconstpointer data)
+{
+ SoupMessage *msg;
+ GUri *uri;
+ GBytes *response;
+ GError *error = NULL;
+
+ uri = g_uri_parse_relative (base_uri, "/close-on-got-headers", SOUP_HTTP_URI_FLAGS, NULL);
+ msg = soup_message_new_from_uri (SOUP_METHOD_GET, uri);
+
+ response = soup_test_session_async_send (test->session, msg, NULL, &error);
+ g_assert_error (error, G_IO_ERROR, G_IO_ERROR_PARTIAL_INPUT);
+
+ g_clear_error (&error);
+ g_bytes_unref (response);
+ g_object_unref (msg);
+ g_uri_unref (uri);
+}
+
static gboolean
unpause_message (SoupServerMessage *msg)
{
@@ -1389,12 +1423,26 @@ server_handler (SoupServer *server,
shutdown (fd, SHUT_WR);
#endif
+ soup_server_message_set_response (msg, "text/plain",
+ SOUP_MEMORY_STATIC,
+ "Success!", 8);
+ } else if (strcmp (path, "/close-on-got-headers") == 0) {
soup_server_message_set_response (msg, "text/plain",
SOUP_MEMORY_STATIC,
"Success!", 8);
}
}
+static void
+server_request_started (SoupServer *server,
+ SoupServerMessage *msg,
+ SoupServerConnection *conn,
+ gpointer user_data)
+{
+ g_signal_connect (msg, "got-headers",
+ G_CALLBACK (disconnect_on_got_headers), NULL);
+}
+
static gboolean
server_basic_auth_callback (SoupAuthDomain *auth_domain,
SoupServerMessage *msg,
@@ -1421,6 +1469,8 @@ main (int argc, char **argv)
return 0;
server = soup_test_server_new (SOUP_TEST_SERVER_IN_THREAD | SOUP_TEST_SERVER_HTTP2);
+ g_signal_connect (server, "request-started",
+ G_CALLBACK (server_request_started), NULL);
auth = soup_auth_domain_basic_new ("realm", "http2-test",
"auth-callback", server_basic_auth_callback,
NULL);
@@ -1577,6 +1627,10 @@ main (int argc, char **argv)
setup_session,
do_broken_pseudo_header_test,
teardown_session);
+ g_test_add ("/http2/server-disconnect-on-got-headers", Test, NULL,
+ setup_session,
+ do_server_disconnect_on_got_headers_test,
+ teardown_session);
ret = g_test_run ();
--
2.54.0