diff options
| author | Daniel P. Berrangé <berrange@redhat.com> | 2024-09-16 13:49:11 +0100 |
|---|---|---|
| committer | Daniel P. Berrangé <berrange@redhat.com> | 2024-10-22 13:02:31 +0100 |
| commit | 1a225f57f3a6bc7a9544b0aa567727f0ef51bc17 (patch) | |
| tree | d4b9dbc225e35419f7926fdcdc501e17ec1967f6 | |
| parent | 829cb3d0eab08e4fea768926f06db1c411a2767f (diff) | |
| download | qemu-1a225f57f3a6bc7a9544b0aa567727f0ef51bc17.zip qemu-1a225f57f3a6bc7a9544b0aa567727f0ef51bc17.tar.gz qemu-1a225f57f3a6bc7a9544b0aa567727f0ef51bc17.tar.bz2 | |
ui: validate NUL byte padding in SASL client data more strictly
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 <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
| -rw-r--r-- | ui/vnc-auth-sasl.c | 20 |
1 files changed, 16 insertions, 4 deletions
diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c index a04feeb..3f4cfc4 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, |