|
| 1 | +From: Daniel P. Berrangé < [email protected]> |
| 2 | +Subject: [PATCH v2 3/3] io: fix use after free in websocket handshake code |
| 3 | +Date: Fri, 3 Oct 2025 16:02:45 +0100 |
| 4 | + |
| 5 | +If the QIOChannelWebsock object is freed while it is waiting to |
| 6 | +complete a handshake, a GSource is leaked. This can lead to the |
| 7 | +callback firing later on and triggering a use-after-free in the |
| 8 | +use of the channel. This was observed in the VNC server with the |
| 9 | +following trace from valgrind: |
| 10 | + |
| 11 | +==2523108== Invalid read of size 4 |
| 12 | +==2523108== at 0x4054A24: vnc_disconnect_start (vnc.c:1296) |
| 13 | +==2523108== by 0x4054A24: vnc_client_error (vnc.c:1392) |
| 14 | +==2523108== by 0x4068A09: vncws_handshake_done (vnc-ws.c:105) |
| 15 | +==2523108== by 0x44863B4: qio_task_complete (task.c:197) |
| 16 | +==2523108== by 0x448343D: qio_channel_websock_handshake_io |
| 17 | +(channel-websock.c:588) |
| 18 | +==2523108== by 0x6EDB862: UnknownInlinedFun (gmain.c:3398) |
| 19 | +==2523108== by 0x6EDB862: g_main_context_dispatch_unlocked.lto_priv.0 |
| 20 | +(gmain.c:4249) |
| 21 | +==2523108== by 0x6EDBAE4: g_main_context_dispatch (gmain.c:4237) |
| 22 | +==2523108== by 0x45EC79F: glib_pollfds_poll (main-loop.c:287) |
| 23 | +==2523108== by 0x45EC79F: os_host_main_loop_wait (main-loop.c:310) |
| 24 | +==2523108== by 0x45EC79F: main_loop_wait (main-loop.c:589) |
| 25 | +==2523108== by 0x423A56D: qemu_main_loop (runstate.c:835) |
| 26 | +==2523108== by 0x454F300: qemu_default_main (main.c:37) |
| 27 | +==2523108== by 0x73D6574: (below main) (libc_start_call_main.h:58) |
| 28 | +==2523108== Address 0x57a6e0dc is 28 bytes inside a block of size 103,608 |
| 29 | +free'd |
| 30 | +==2523108== at 0x5F2FE43: free (vg_replace_malloc.c:989) |
| 31 | +==2523108== by 0x6EDC444: g_free (gmem.c:208) |
| 32 | +==2523108== by 0x4053F23: vnc_update_client (vnc.c:1153) |
| 33 | +==2523108== by 0x4053F23: vnc_refresh (vnc.c:3225) |
| 34 | +==2523108== by 0x4042881: dpy_refresh (console.c:880) |
| 35 | +==2523108== by 0x4042881: gui_update (console.c:90) |
| 36 | +==2523108== by 0x45EFA1B: timerlist_run_timers.part.0 (qemu-timer.c:562) |
| 37 | +==2523108== by 0x45EFC8F: timerlist_run_timers (qemu-timer.c:495) |
| 38 | +==2523108== by 0x45EFC8F: qemu_clock_run_timers (qemu-timer.c:576) |
| 39 | +==2523108== by 0x45EFC8F: qemu_clock_run_all_timers (qemu-timer.c:663) |
| 40 | +==2523108== by 0x45EC765: main_loop_wait (main-loop.c:600) |
| 41 | +==2523108== by 0x423A56D: qemu_main_loop (runstate.c:835) |
| 42 | +==2523108== by 0x454F300: qemu_default_main (main.c:37) |
| 43 | +==2523108== by 0x73D6574: (below main) (libc_start_call_main.h:58) |
| 44 | +==2523108== Block was alloc'd at |
| 45 | +==2523108== at 0x5F343F3: calloc (vg_replace_malloc.c:1675) |
| 46 | +==2523108== by 0x6EE2F81: g_malloc0 (gmem.c:133) |
| 47 | +==2523108== by 0x4057DA3: vnc_connect (vnc.c:3245) |
| 48 | +==2523108== by 0x448591B: qio_net_listener_channel_func (net-listener.c:54) |
| 49 | +==2523108== by 0x6EDB862: UnknownInlinedFun (gmain.c:3398) |
| 50 | +==2523108== by 0x6EDB862: g_main_context_dispatch_unlocked.lto_priv.0 |
| 51 | +(gmain.c:4249) |
| 52 | +==2523108== by 0x6EDBAE4: g_main_context_dispatch (gmain.c:4237) |
| 53 | +==2523108== by 0x45EC79F: glib_pollfds_poll (main-loop.c:287) |
| 54 | +==2523108== by 0x45EC79F: os_host_main_loop_wait (main-loop.c:310) |
| 55 | +==2523108== by 0x45EC79F: main_loop_wait (main-loop.c:589) |
| 56 | +==2523108== by 0x423A56D: qemu_main_loop (runstate.c:835) |
| 57 | +==2523108== by 0x454F300: qemu_default_main (main.c:37) |
| 58 | +==2523108== by 0x73D6574: (below main) (libc_start_call_main.h:58) |
| 59 | +==2523108== |
| 60 | + |
| 61 | +The above can be reproduced by launching QEMU with |
| 62 | + |
| 63 | + $ qemu-system-x86_64 -vnc localhost:0,websocket=5700 |
| 64 | + |
| 65 | +and then repeatedly running: |
| 66 | + |
| 67 | + for i in {1..100}; do |
| 68 | + (echo -n "GET / HTTP/1.1" && sleep 0.05) | nc -w 1 localhost 5700 & |
| 69 | + done |
| 70 | + |
| 71 | +CVE-2025-11234 |
| 72 | +Reported-by: Grant Millar | Cylo < [email protected]> |
| 73 | +Signed-off-by: Daniel P. Berrangé < [email protected]> |
| 74 | + |
| 75 | +Upstream Patch reference: |
| 76 | +1. https://lists.nongnu.org/archive/html/qemu-devel/2025-10/msg00784.html |
| 77 | +2. https://lists.nongnu.org/archive/html/qemu-devel/2025-10/msg00785.html |
| 78 | +3. https://lists.nongnu.org/archive/html/qemu-devel/2025-10/msg00786.html |
| 79 | +--- |
| 80 | + include/io/channel-websock.h | 3 ++- |
| 81 | + io/channel-tls.c | 5 +++++ |
| 82 | + io/channel-websock.c | 33 ++++++++++++++++++++++++++------- |
| 83 | + io/trace-events | 1 + |
| 84 | + 4 files changed, 34 insertions(+), 8 deletions(-) |
| 85 | + |
| 86 | +diff --git a/include/io/channel-websock.h b/include/io/channel-websock.h |
| 87 | +index e180827c5..6700cf894 100644 |
| 88 | +--- a/include/io/channel-websock.h |
| 89 | ++++ b/include/io/channel-websock.h |
| 90 | +@@ -61,7 +61,8 @@ struct QIOChannelWebsock { |
| 91 | + size_t payload_remain; |
| 92 | + size_t pong_remain; |
| 93 | + QIOChannelWebsockMask mask; |
| 94 | +- guint io_tag; |
| 95 | ++ guint hs_io_tag; /* tracking handshake task */ |
| 96 | ++ guint io_tag; /* tracking watch task */ |
| 97 | + Error *io_err; |
| 98 | + gboolean io_eof; |
| 99 | + uint8_t opcode; |
| 100 | +diff --git a/io/channel-tls.c b/io/channel-tls.c |
| 101 | +index 58fe1acee..bf8ac223d 100644 |
| 102 | +--- a/io/channel-tls.c |
| 103 | ++++ b/io/channel-tls.c |
| 104 | +@@ -255,6 +255,11 @@ static void qio_channel_tls_finalize(Object *obj) |
| 105 | + { |
| 106 | + QIOChannelTLS *ioc = QIO_CHANNEL_TLS(obj); |
| 107 | + |
| 108 | ++ if (ioc->hs_ioc_tag) { |
| 109 | ++ trace_qio_channel_tls_handshake_cancel(ioc); |
| 110 | ++ g_clear_handle_id(&ioc->hs_ioc_tag, g_source_remove); |
| 111 | ++ } |
| 112 | ++ |
| 113 | + object_unref(OBJECT(ioc->master)); |
| 114 | + qcrypto_tls_session_free(ioc->session); |
| 115 | + } |
| 116 | +diff --git a/io/channel-websock.c b/io/channel-websock.c |
| 117 | +index a12acc27c..594e647c8 100644 |
| 118 | +--- a/io/channel-websock.c |
| 119 | ++++ b/io/channel-websock.c |
| 120 | +@@ -545,6 +545,7 @@ static gboolean qio_channel_websock_handshake_send(QIOChannel *ioc, |
| 121 | + trace_qio_channel_websock_handshake_fail(ioc, error_get_pretty(err)); |
| 122 | + qio_task_set_error(task, err); |
| 123 | + qio_task_complete(task); |
| 124 | ++ wioc->hs_io_tag = 0; |
| 125 | + return FALSE; |
| 126 | + } |
| 127 | + |
| 128 | +@@ -560,6 +561,7 @@ static gboolean qio_channel_websock_handshake_send(QIOChannel *ioc, |
| 129 | + trace_qio_channel_websock_handshake_complete(ioc); |
| 130 | + qio_task_complete(task); |
| 131 | + } |
| 132 | ++ wioc->hs_io_tag = 0; |
| 133 | + return FALSE; |
| 134 | + } |
| 135 | + trace_qio_channel_websock_handshake_pending(ioc, G_IO_OUT); |
| 136 | +@@ -586,6 +588,7 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel *ioc, |
| 137 | + trace_qio_channel_websock_handshake_fail(ioc, error_get_pretty(err)); |
| 138 | + qio_task_set_error(task, err); |
| 139 | + qio_task_complete(task); |
| 140 | ++ wioc->hs_io_tag = 0; |
| 141 | + return FALSE; |
| 142 | + } |
| 143 | + if (ret == 0) { |
| 144 | +@@ -597,7 +600,7 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel *ioc, |
| 145 | + error_propagate(&wioc->io_err, err); |
| 146 | + |
| 147 | + trace_qio_channel_websock_handshake_reply(ioc); |
| 148 | +- qio_channel_add_watch( |
| 149 | ++ wioc->hs_io_tag = qio_channel_add_watch( |
| 150 | + wioc->master, |
| 151 | + G_IO_OUT, |
| 152 | + qio_channel_websock_handshake_send, |
| 153 | +@@ -906,11 +909,12 @@ void qio_channel_websock_handshake(QIOChannelWebsock *ioc, |
| 154 | + |
| 155 | + trace_qio_channel_websock_handshake_start(ioc); |
| 156 | + trace_qio_channel_websock_handshake_pending(ioc, G_IO_IN); |
| 157 | +- qio_channel_add_watch(ioc->master, |
| 158 | +- G_IO_IN, |
| 159 | +- qio_channel_websock_handshake_io, |
| 160 | +- task, |
| 161 | +- NULL); |
| 162 | ++ ioc->hs_io_tag = qio_channel_add_watch( |
| 163 | ++ ioc->master, |
| 164 | ++ G_IO_IN, |
| 165 | ++ qio_channel_websock_handshake_io, |
| 166 | ++ task, |
| 167 | ++ NULL); |
| 168 | + } |
| 169 | + |
| 170 | + |
| 171 | +@@ -921,13 +925,16 @@ static void qio_channel_websock_finalize(Object *obj) |
| 172 | + buffer_free(&ioc->encinput); |
| 173 | + buffer_free(&ioc->encoutput); |
| 174 | + buffer_free(&ioc->rawinput); |
| 175 | +- object_unref(OBJECT(ioc->master)); |
| 176 | ++ if (ioc->hs_io_tag) { |
| 177 | ++ g_source_remove(ioc->hs_io_tag); |
| 178 | ++ } |
| 179 | + if (ioc->io_tag) { |
| 180 | + g_source_remove(ioc->io_tag); |
| 181 | + } |
| 182 | + if (ioc->io_err) { |
| 183 | + error_free(ioc->io_err); |
| 184 | + } |
| 185 | ++ object_unref(OBJECT(ioc->master)); |
| 186 | + } |
| 187 | + |
| 188 | + |
| 189 | +@@ -1218,6 +1225,18 @@ static int qio_channel_websock_close(QIOChannel *ioc, |
| 190 | + QIOChannelWebsock *wioc = QIO_CHANNEL_WEBSOCK(ioc); |
| 191 | + |
| 192 | + trace_qio_channel_websock_close(ioc); |
| 193 | ++ buffer_free(&wioc->encinput); |
| 194 | ++ buffer_free(&wioc->encoutput); |
| 195 | ++ buffer_free(&wioc->rawinput); |
| 196 | ++ if (wioc->hs_io_tag) { |
| 197 | ++ g_clear_handle_id(&wioc->hs_io_tag, g_source_remove); |
| 198 | ++ } |
| 199 | ++ if (wioc->io_tag) { |
| 200 | ++ g_clear_handle_id(&wioc->io_tag, g_source_remove); |
| 201 | ++ } |
| 202 | ++ if (wioc->io_err) { |
| 203 | ++ g_clear_pointer(&wioc->io_err, error_free); |
| 204 | ++ } |
| 205 | + return qio_channel_close(wioc->master, errp); |
| 206 | + } |
| 207 | + |
| 208 | +diff --git a/io/trace-events b/io/trace-events |
| 209 | +index 3cc5cf1ef..d4c0f84a9 100644 |
| 210 | +--- a/io/trace-events |
| 211 | ++++ b/io/trace-events |
| 212 | +@@ -43,6 +43,7 @@ qio_channel_tls_handshake_start(void *ioc) "TLS handshake start ioc=%p" |
| 213 | + qio_channel_tls_handshake_pending(void *ioc, int status) "TLS handshake pending ioc=%p status=%d" |
| 214 | + qio_channel_tls_handshake_fail(void *ioc) "TLS handshake fail ioc=%p" |
| 215 | + qio_channel_tls_handshake_complete(void *ioc) "TLS handshake complete ioc=%p" |
| 216 | ++qio_channel_tls_handshake_cancel(void *ioc) "TLS handshake cancel ioc=%p" |
| 217 | + qio_channel_tls_credentials_allow(void *ioc) "TLS credentials allow ioc=%p" |
| 218 | + qio_channel_tls_credentials_deny(void *ioc) "TLS credentials deny ioc=%p" |
| 219 | + |
| 220 | +-- |
| 221 | +2.45.4 |
| 222 | + |
0 commit comments