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.
pull/45/head v0.3.0
Scott Moreau 2020-09-18 17:15:57 -06:00 committed by Andri Yngvason
parent 6d29937e15
commit 783807c0b9
1 changed files with 5 additions and 2 deletions

View File

@ -618,6 +618,9 @@ static int on_client_cut_text(struct nvnc_client* client)
return 0; return 0;
} }
if (client->buffer_len - client->buffer_index < sizeof(*msg) + length)
return 0;
nvnc_cut_text_fn fn = server->cut_text_fn; nvnc_cut_text_fn fn = server->cut_text_fn;
if (fn) { if (fn) {
fn(server, msg->text, length); 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); 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; client->buffer_len -= client->buffer_index;
memmove(client->msg_buffer, client->msg_buffer + client->buffer_index,
client->buffer_len);
client->buffer_index = 0; client->buffer_index = 0;
} }