From bdcade47224f154052c927aed3c363a18b37112e Mon Sep 17 00:00:00 2001
From: Azat Khuzhin <azat@libevent.org>
Date: Wed, 31 Jul 2019 10:34:38 +0300
Subject: [PATCH] buffer: fix possible NULL dereference in evbuffer_setcb() on
 ENOMEM

[ @azat:

  - add return heredoc for evbuffer_setcb()
  - add unit test with event_set_mem_functions()
  - look through the report from abi-compliance-checker/abi-dumper
]

Closes: #855
---
 buffer.c                       |  7 ++++++-
 include/event2/buffer_compat.h |  3 ++-
 test/regress_buffer.c          | 12 ++++++++++--
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/buffer.c b/buffer.c
index 6ad8fd24..01418f5a 100644
--- a/buffer.c
+++ b/buffer.c
@@ -3321,7 +3321,7 @@ evbuffer_add_file(struct evbuffer *buf, int fd, ev_off_t offset, ev_off_t length
 	return r;
 }
 
-void
+int
 evbuffer_setcb(struct evbuffer *buffer, evbuffer_cb cb, void *cbarg)
 {
 	EVBUFFER_LOCK(buffer);
@@ -3332,10 +3332,15 @@ evbuffer_setcb(struct evbuffer *buffer, evbuffer_cb cb, void *cbarg)
 	if (cb) {
 		struct evbuffer_cb_entry *ent =
 		    evbuffer_add_cb(buffer, NULL, cbarg);
+		if (!ent) {
+			EVBUFFER_UNLOCK(buffer);
+			return -1;
+		}
 		ent->cb.cb_obsolete = cb;
 		ent->flags |= EVBUFFER_CB_OBSOLETE;
 	}
 	EVBUFFER_UNLOCK(buffer);
+	return 0;
 }
 
 struct evbuffer_cb_entry *
diff --git a/include/event2/buffer_compat.h b/include/event2/buffer_compat.h
index 24f828c2..0ce10254 100644
--- a/include/event2/buffer_compat.h
+++ b/include/event2/buffer_compat.h
@@ -90,9 +90,10 @@ typedef void (*evbuffer_cb)(struct evbuffer *buffer, size_t old_len, size_t new_
   @param cb the callback function to invoke when the evbuffer is modified,
 	 or NULL to remove all callbacks.
   @param cbarg an argument to be provided to the callback function
+  @return 0 if successful, or -1 on error
  */
 EVENT2_EXPORT_SYMBOL
-void evbuffer_setcb(struct evbuffer *buffer, evbuffer_cb cb, void *cbarg);
+int evbuffer_setcb(struct evbuffer *buffer, evbuffer_cb cb, void *cbarg);
 
 
 /**
diff --git a/test/regress_buffer.c b/test/regress_buffer.c
index 02d557b6..8ac4b6e0 100644
--- a/test/regress_buffer.c
+++ b/test/regress_buffer.c
@@ -1956,12 +1956,12 @@ test_evbuffer_callbacks(void *ptr)
 	tt_assert(cb1 != NULL);
 	cb2 = evbuffer_add_cb(buf, log_change_callback, buf_out2);
 	tt_assert(cb2 != NULL);
-	evbuffer_setcb(buf, self_draining_callback, NULL);
+	tt_int_op(evbuffer_setcb(buf, self_draining_callback, NULL), ==, 0);
 	evbuffer_add_printf(buf, "This should get drained right away.");
 	tt_uint_op(evbuffer_get_length(buf), ==, 0);
 	tt_uint_op(evbuffer_get_length(buf_out1), ==, 0);
 	tt_uint_op(evbuffer_get_length(buf_out2), ==, 0);
-	evbuffer_setcb(buf, NULL, NULL);
+	tt_int_op(evbuffer_setcb(buf, NULL, NULL), ==, 0);
 	evbuffer_add_printf(buf, "This will not.");
 	tt_str_op((const char *) evbuffer_pullup(buf, -1), ==, "This will not.");
 	evbuffer_validate(buf);
@@ -1987,6 +1987,14 @@ test_evbuffer_callbacks(void *ptr)
 		  "0->15; 15->11; 11->0; ");
 #endif
 
+	/* the next call to readline should fail */
+#ifndef EVENT__DISABLE_MM_REPLACEMENT
+	event_set_mem_functions(failing_malloc, realloc, free);
+	tt_int_op(evbuffer_setcb(buf, self_draining_callback, NULL), ==, -1);
+	evbuffer_validate(buf);
+	event_set_mem_functions(malloc, realloc, free);
+#endif
+
  end:
 	if (buf)
 		evbuffer_free(buf);
-- 
2.40.0