From 0ef3520a3cef658dc208510d7256f9f36bad1d88 Mon Sep 17 00:00:00 2001
From: "Todd C. Miller" <Todd.Miller@sudo.ws>
Date: Wed, 1 Mar 2023 13:58:32 -0700
Subject: Check for sudo_pow2_roundup() overflow. Calling
 sudo_pow2_roundup(INT_MAX+2) will return since there is no power of 2 larger
 than INT_MAX+1 that fits in an unsigned int. This is not an issue in practice
 since we restrict messages to 2Mib.

---
 logsrvd/logsrv_util.c     | 18 ++++++++++++------
 logsrvd/logsrvd.c         | 28 ++++++++++++++++++----------
 logsrvd/logsrvd_journal.c |  5 +++++
 logsrvd/sendlog.c         | 15 ++++++++++++---
 4 files changed, 47 insertions(+), 19 deletions(-)

(limited to 'logsrvd')

diff --git a/logsrvd/logsrv_util.c b/logsrvd/logsrv_util.c
index f70604d52..118b81bb4 100644
--- a/logsrvd/logsrv_util.c
+++ b/logsrvd/logsrv_util.c
@@ -62,18 +62,21 @@ expand_buf(struct connection_buffer *buf, unsigned int needed)
 
     if (buf->size < needed) {
 	/* Expand buffer. */
-	needed = sudo_pow2_roundup(needed);
+	const unsigned int newsize = sudo_pow2_roundup(needed);
 	sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
-	    "expanding buffer from %u to %u", buf->size, needed);
-	if ((newdata = malloc(needed)) == NULL) {
-	    sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
-	    debug_return_bool(false);
+	    "expanding buffer from %u to %u", buf->size, newsize);
+	if (newsize < needed) {
+	    /* overflow */
+	    errno = ENOMEM;
+	    goto oom;
 	}
+	if ((newdata = malloc(newsize)) == NULL)
+	    goto oom;
 	if (buf->len != buf->off)
 	    memcpy(newdata, buf->data + buf->off, buf->len - buf->off);
 	free(buf->data);
 	buf->data = newdata;
-	buf->size = needed;
+	buf->size = newsize;
     } else {
 	/* Just reset existing buffer. */
 	if (buf->len != buf->off) {
@@ -85,6 +88,9 @@ expand_buf(struct connection_buffer *buf, unsigned int needed)
     buf->off = 0;
 
     debug_return_bool(true);
+oom:
+    sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
+    debug_return_bool(false);
 }
 
 /*
diff --git a/logsrvd/logsrvd.c b/logsrvd/logsrvd.c
index 17edb84a5..891ce8115 100644
--- a/logsrvd/logsrvd.c
+++ b/logsrvd/logsrvd.c
@@ -297,23 +297,31 @@ get_free_buf(size_t len, struct connection_closure *closure)
     if (buf != NULL) {
         TAILQ_REMOVE(&closure->free_bufs, buf, entries);
     } else {
-        if ((buf = calloc(1, sizeof(*buf))) == NULL) {
-	    sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
-	    debug_return_ptr(NULL);
-	}
+        if ((buf = calloc(1, sizeof(*buf))) == NULL)
+	    goto oom;
     }
 
     if (len > buf->size) {
-	free(buf->data);
-	buf->size = sudo_pow2_roundup(len);
-	if ((buf->data = malloc(buf->size)) == NULL) {
-	    sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
-	    free(buf);
-	    buf = NULL;
+	const unsigned int new_size = sudo_pow2_roundup(len);
+	if (new_size < len) {
+	    /* overflow */
+	    errno = ENOMEM;
+	    goto oom;
 	}
+	free(buf->data);
+	if ((buf->data = malloc(new_size)) == NULL)
+	    goto oom;
+	buf->size = new_size;
     }
 
     debug_return_ptr(buf);
+oom:
+    if (buf != NULL) {
+	free(buf->data);
+	free(buf);
+    }
+    sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
+    debug_return_ptr(NULL);
 }
 
 static bool
diff --git a/logsrvd/logsrvd_journal.c b/logsrvd/logsrvd_journal.c
index 354d71c80..0983d1f67 100644
--- a/logsrvd/logsrvd_journal.c
+++ b/logsrvd/logsrvd_journal.c
@@ -268,6 +268,11 @@ journal_seek(struct timespec *target, struct connection_closure *closure)
 
 	    if (msg_len > bufsize) {
 		bufsize = sudo_pow2_roundup(msg_len);
+		if (bufsize < msg_len) {
+		    /* overflow */
+		    closure->errstr = _("unable to allocate memory");
+		    break;
+		}
 		free(buf);
 		if ((buf = malloc(bufsize)) == NULL) {
 		    closure->errstr = _("unable to allocate memory");
diff --git a/logsrvd/sendlog.c b/logsrvd/sendlog.c
index 98d5ecc10..ebf6c9d7b 100644
--- a/logsrvd/sendlog.c
+++ b/logsrvd/sendlog.c
@@ -256,7 +256,7 @@ get_free_buf(size_t len, struct client_closure *closure)
     if (len > buf->size) {
 	free(buf->data);
 	buf->size = sudo_pow2_roundup(len);
-	if ((buf->data = malloc(buf->size)) == NULL) {
+	if (buf->size < len || (buf->data = malloc(buf->size)) == NULL) {
 	    sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
 	    free(buf);
 	    buf = NULL;
@@ -285,13 +285,22 @@ read_io_buf(struct client_closure *closure)
 
     /* Expand buf as needed. */
     if (timing->u.nbytes > closure->bufsize) {
+	const size_t new_size = sudo_pow2_roundup(timing->u.nbytes);
+	if (new_size < timing->u.nbytes) {
+	    /* overflow */
+	    errno = ENOMEM;
+	    sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
+	    timing->u.nbytes = 0;
+	    debug_return_bool(false);
+	}
 	free(closure->buf);
-	closure->bufsize = sudo_pow2_roundup(timing->u.nbytes);
-	if ((closure->buf = malloc(closure->bufsize)) == NULL) {
+	if ((closure->buf = malloc(new_size)) == NULL) {
 	    sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
+	    closure->bufsize = 0;
 	    timing->u.nbytes = 0;
 	    debug_return_bool(false);
 	}
+	closure->bufsize = new_size;
     }
 
     nread = iolog_read(&closure->iolog_files[timing->event], closure->buf,
-- 
cgit v1.2.1