]> granicus.if.org Git - libevent/commitdiff
buffer: do not pass NULL to memcpy() from evbuffer_pullup()
authorAzat Khuzhin <azat@libevent.org>
Thu, 25 Jun 2020 07:15:01 +0000 (10:15 +0300)
committerAzat Khuzhin <azat@libevent.org>
Thu, 25 Jun 2020 07:15:01 +0000 (10:15 +0300)
UBSAN reports:

  evbuffer/remove_buffer_with_empty3: ../buffer.c:1443:3: runtime error: null pointer passed as argument 2, which is declared to never be null
      #0 0x7ffff6cd0410 in evbuffer_pullup ../buffer.c:1443
      #1 0x5555556d68b9 in test_evbuffer_remove_buffer_with_empty3 ../test/regress_buffer.c:408
      #2 0x5555557b95ee in testcase_run_bare_ ../test/tinytest.c:173
      #3 0x5555557ba048 in testcase_run_one ../test/tinytest.c:333
      #4 0x5555557bc0f8 in tinytest_main ../test/tinytest.c:527
      #5 0x555555787702 in main ../test/regress_main.c:528
      #6 0x7ffff606c001 in __libc_start_main (/usr/lib/libc.so.6+0x27001)
      #7 0x55555569436d in _start (/src/le/libevent/.cmake-debug/bin/regress+0x14036d)

buffer.c
test/regress_buffer.c

index bcb246f68ca59069badc9e155e46104f71b99982..121b37c009ae76e8f91c8a5d59c3b781fd5dec44 100644 (file)
--- a/buffer.c
+++ b/buffer.c
@@ -1440,9 +1440,11 @@ evbuffer_pullup(struct evbuffer *buf, ev_ssize_t size)
        for (; chain != NULL && (size_t)size >= chain->off; chain = next) {
                next = chain->next;
 
-               memcpy(buffer, chain->buffer + chain->misalign, chain->off);
-               size -= chain->off;
-               buffer += chain->off;
+               if (chain->buffer) {
+                       memcpy(buffer, chain->buffer + chain->misalign, chain->off);
+                       size -= chain->off;
+                       buffer += chain->off;
+               }
                if (chain == last_with_data)
                        removed_last_with_data = 1;
                if (&chain->next == buf->last_with_datap)
index b4c45edc69cf1c79d95bb5fa9a1c9847f0ecc0b1..6d55aead526de2384ccd4e512e983a09d8b41935 100644 (file)
@@ -425,6 +425,36 @@ test_evbuffer_remove_buffer_with_empty3(void *ptr)
        evbuffer_free(buf);
 }
 
+static void
+test_evbuffer_pullup_with_empty(void *ptr)
+{
+       struct evbuffer *buf = NULL;
+
+       buf = evbuffer_new();
+       evbuffer_add(buf, "foo", 3);
+       evbuffer_add_reference(buf, NULL, 0, NULL, NULL);
+       evbuffer_validate(buf);
+       tt_int_op(evbuffer_get_length(buf), ==, 3);
+       tt_mem_op(evbuffer_pullup(buf, -1), ==, "foo", 3);
+
+       evbuffer_free(buf);
+       buf = evbuffer_new();
+       evbuffer_validate(buf);
+       tt_int_op(evbuffer_get_length(buf), ==, 0);
+       tt_int_op(evbuffer_pullup(buf, -1), ==, NULL);
+
+       evbuffer_free(buf);
+       buf = evbuffer_new();
+       evbuffer_add(buf, "foo", 3);
+       evbuffer_add_reference(buf, NULL, 0, NULL, NULL);
+       evbuffer_validate(buf);
+       tt_mem_op(evbuffer_pullup(buf, 3), ==, "foo", 3);
+
+ end:
+       if (buf)
+               evbuffer_free(buf);
+}
+
 static void
 test_evbuffer_remove_buffer_with_empty_front(void *ptr)
 {
@@ -2810,6 +2840,7 @@ struct testcase_t evbuffer_testcases[] = {
        { "add_iovec", test_evbuffer_add_iovec, 0, NULL, NULL},
        { "copyout", test_evbuffer_copyout, 0, NULL, NULL},
        { "file_segment_add_cleanup_cb", test_evbuffer_file_segment_add_cleanup_cb, 0, NULL, NULL },
+       { "pullup_with_empty", test_evbuffer_pullup_with_empty, 0, NULL, NULL },
 
 #define ADDFILE_TEST(name, parameters)                                 \
        { name, test_evbuffer_add_file, TT_FORK|TT_NEED_BASE,           \