wsutil: fix crash due to corruption of the "small_buffers" array
authorPeter Wu <peter@lekensteyn.nl>
Mon, 31 Dec 2018 13:07:32 +0000 (14:07 +0100)
committerAnders Broman <a.broman58@gmail.com>
Wed, 2 Jan 2019 08:11:37 +0000 (08:11 +0000)
Gracefully handle repeated calls of ws_buffer_free on the same buffer to
avoid strange crashes in other new users that allocate a "small" buffer.

The first call to ws_buffer_free would store data pointer in the
'small_buffers' array for reuse and set the pointer to NULL. Result:

    (gdb) p cfile.rec.options_buf
    $2 = {
      data = 0x0,
      allocated = 2048,     // Oops, not modified!
      start = 0,
      first_free = 0
    }

All users of Buffer (including ws_buffer_free) however asssume that
'allocated' reflects the actual size of 'data'. If this is not the case
(if ws_buffer_free is called again), then a data pointer (NULL!) will be
stored and the next ws_buffer_init request for a "small buffer" will
result in unexpected behavior (including crashes).

Fix the issue by clearing the 'allocated' field as well. Add assertions
to catch such issues earlier rather than crashing at random users of
these buffers (such as frame_tvbuff).

Bug: 15263
Change-Id: I0b491c3fccac8c6fddd43779629343d721638ca9
Reviewed-on: https://code.wireshark.org/review/31278
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
wsutil/buffer.c

index bbe0ed11d21627987e7a81d416da4b77ae41d722..d28bcfe84b01f88cb3831626f8280f81ac918fcc 100644 (file)
@@ -26,6 +26,7 @@ ws_buffer_init(Buffer* buffer, gsize space)
        if (space <= SMALL_BUFFER_SIZE) {
                if (small_buffers->len > 0) {
                        buffer->data = (guint8*) g_ptr_array_remove_index(small_buffers, small_buffers->len - 1);
+                       g_assert(buffer->data);
                } else {
                        buffer->data = (guint8*)g_malloc(SMALL_BUFFER_SIZE);
                }
@@ -44,10 +45,12 @@ ws_buffer_free(Buffer* buffer)
 {
        g_assert(buffer);
        if (buffer->allocated == SMALL_BUFFER_SIZE) {
+               g_assert(buffer->data);
                g_ptr_array_add(small_buffers, buffer->data);
        } else {
                g_free(buffer->data);
        }
+       buffer->allocated = 0;
        buffer->data = NULL;
 }