From 1a225f57f3a6bc7a9544b0aa567727f0ef51bc17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 16 Sep 2024 13:49:11 +0100 Subject: [PATCH] ui: validate NUL byte padding in SASL client data more strictly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the SASL data is non-NULL, the SASL protocol spec requires that it is padded with a trailing NUL byte. QEMU discards the trailing byte, but does not currently validate that it was in fact a NUL. Apply strict validation to better detect any broken clients. Reviewed-by: Marc-André Lureau Signed-off-by: Daniel P. Berrangé --- ui/vnc-auth-sasl.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c index a04feeb429..3f4cfc471d 100644 --- a/ui/vnc-auth-sasl.c +++ b/ui/vnc-auth-sasl.c @@ -263,8 +263,14 @@ static int protocol_client_auth_sasl_step(VncState *vs, uint8_t *data, size_t le /* NB, distinction of NULL vs "" is *critical* in SASL */ if (datalen) { clientdata = (char*)data; - clientdata[datalen-1] = '\0'; /* Wire includes '\0', but make sure */ - datalen--; /* Don't count NULL byte when passing to _start() */ + if (clientdata[datalen - 1] != '\0') { + trace_vnc_auth_fail(vs, vs->auth, "Malformed SASL client data", + "Missing SASL NUL padding byte"); + sasl_dispose(&vs->sasl.conn); + vs->sasl.conn = NULL; + goto authabort; + } + datalen--; /* Discard the extra NUL padding byte */ } err = sasl_server_step(vs->sasl.conn, @@ -385,8 +391,14 @@ static int protocol_client_auth_sasl_start(VncState *vs, uint8_t *data, size_t l /* NB, distinction of NULL vs "" is *critical* in SASL */ if (datalen) { clientdata = (char*)data; - clientdata[datalen-1] = '\0'; /* Should be on wire, but make sure */ - datalen--; /* Don't count NULL byte when passing to _start() */ + if (clientdata[datalen - 1] != '\0') { + trace_vnc_auth_fail(vs, vs->auth, "Malformed SASL client data", + "Missing SASL NUL padding byte"); + sasl_dispose(&vs->sasl.conn); + vs->sasl.conn = NULL; + goto authabort; + } + datalen--; /* Discard the extra NUL padding byte */ } err = sasl_server_start(vs->sasl.conn,