From 783807c0b933d2d28d41fe6ece39a51f4b341e09 Mon Sep 17 00:00:00 2001 From: Scott Moreau Date: Fri, 18 Sep 2020 17:15:57 -0600 Subject: [PATCH] server: Fix possible crash on fragmented packet messages The packets sent from the client especially for client cut text, are typically sent in two packets, one for the message containing the type and length and the other for the actual data. Sometimes the first message is read but we still don't have the data yet. We need to continue reading data to use the structure but this revealed a bug. The client event handler was calling memmove() with buffer_index as the size argument. This meant that it was copying the wrong amount of data, resulting in garbage at the end of the expected data. This patch fixes the problem by first subtracting buffer_index from buffer_len and then moving buffer_len worth of data, which is what was read into msg_buffer. The problem possibly manifested itself with random crashes, after reading random data. --- src/server.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/server.c b/src/server.c index 2f12689..d3f0ab9 100644 --- a/src/server.c +++ b/src/server.c @@ -618,6 +618,9 @@ static int on_client_cut_text(struct nvnc_client* client) return 0; } + if (client->buffer_len - client->buffer_index < sizeof(*msg) + length) + return 0; + nvnc_cut_text_fn fn = server->cut_text_fn; if (fn) { fn(server, msg->text, length); @@ -729,9 +732,9 @@ static void on_client_event(struct stream* stream, enum stream_event event) assert(client->buffer_index <= client->buffer_len); - memmove(client->msg_buffer, client->msg_buffer + client->buffer_index, - client->buffer_index); client->buffer_len -= client->buffer_index; + memmove(client->msg_buffer, client->msg_buffer + client->buffer_index, + client->buffer_len); client->buffer_index = 0; }