On Wed, Jul 5, 2023 at 10:20 PM Daniel P. Berrangé <berra...@redhat.com>
wrote:
> The TLS handshake make take some time to complete, during which time an
> I/O watch might be registered with the main loop. If the owner of the
> I/O channel invokes qio_channel_close() while the handshake is waiting
> to continue the I/O watch must be removed. Failing to remove it will
> later trigger the completion callback which the owner is not expecting
> to receive. In the case of the VNC server, this results in a SEGV as
> vnc_disconnect_start() tries to shutdown a client connection that is
> already gone / NULL.
>
> CVE-2023-3354
> Reported-by: jiangyegen <jiangye...@huawei.com>
> Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
>
> ---
> include/io/channel-tls.h | 1 +
> io/channel-tls.c | 18 ++++++++++++------
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/include/io/channel-tls.h b/include/io/channel-tls.h
> index 5672479e9e..26c67f17e2 100644
> --- a/include/io/channel-tls.h
> +++ b/include/io/channel-tls.h
> @@ -48,6 +48,7 @@ struct QIOChannelTLS {
> QIOChannel *master;
> QCryptoTLSSession *session;
> QIOChannelShutdown shutdown;
> + guint hs_ioc_tag;
> };
>
> /**
> diff --git a/io/channel-tls.c b/io/channel-tls.c
> index 9805dd0a3f..e327e6a5c2 100644
> --- a/io/channel-tls.c
> +++ b/io/channel-tls.c
> @@ -198,12 +198,13 @@ static void
> qio_channel_tls_handshake_task(QIOChannelTLS *ioc,
> }
>
> trace_qio_channel_tls_handshake_pending(ioc, status);
> - qio_channel_add_watch_full(ioc->master,
> - condition,
> - qio_channel_tls_handshake_io,
> - data,
> - NULL,
> - context);
> + ioc->hs_ioc_tag =
> + qio_channel_add_watch_full(ioc->master,
> + condition,
> + qio_channel_tls_handshake_io,
> + data,
> + NULL,
> + context);
> }
> }
>
> @@ -218,6 +219,7 @@ static gboolean
> qio_channel_tls_handshake_io(QIOChannel *ioc,
> QIOChannelTLS *tioc = QIO_CHANNEL_TLS(
> qio_task_get_source(task));
>
> + tioc->hs_ioc_tag = 0;
> g_free(data);
> qio_channel_tls_handshake_task(tioc, task, context);
>
> @@ -378,6 +380,10 @@ static int qio_channel_tls_close(QIOChannel *ioc,
> {
> QIOChannelTLS *tioc = QIO_CHANNEL_TLS(ioc);
>
> + if (tioc->hs_ioc_tag) {
> + g_source_remove(tioc->hs_ioc_tag);
>
set it to 0 ?
or
g_clear_handle_id(&tios->hs_ioc_tag, g_source_remove);
> + }
> +
> return qio_channel_close(tioc->master, errp);
> }
>
> --
> 2.41.0
>
>
>
--
Marc-André Lureau
