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.
This commit is contained in:
Todd C. Miller
2023-03-01 13:58:32 -07:00
parent 19a660612f
commit b013711e48
5 changed files with 69 additions and 29 deletions

View File

@@ -62,18 +62,21 @@ expand_buf(struct connection_buffer *buf, unsigned int needed)
if (buf->size < needed) { if (buf->size < needed) {
/* Expand buffer. */ /* Expand buffer. */
needed = sudo_pow2_roundup(needed); const unsigned int newsize = sudo_pow2_roundup(needed);
sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO, sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
"expanding buffer from %u to %u", buf->size, needed); "expanding buffer from %u to %u", buf->size, newsize);
if ((newdata = malloc(needed)) == NULL) { if (newsize < needed) {
sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); /* overflow */
debug_return_bool(false); errno = ENOMEM;
goto oom;
} }
if ((newdata = malloc(newsize)) == NULL)
goto oom;
if (buf->len != buf->off) if (buf->len != buf->off)
memcpy(newdata, buf->data + buf->off, buf->len - buf->off); memcpy(newdata, buf->data + buf->off, buf->len - buf->off);
free(buf->data); free(buf->data);
buf->data = newdata; buf->data = newdata;
buf->size = needed; buf->size = newsize;
} else { } else {
/* Just reset existing buffer. */ /* Just reset existing buffer. */
if (buf->len != buf->off) { if (buf->len != buf->off) {
@@ -85,6 +88,9 @@ expand_buf(struct connection_buffer *buf, unsigned int needed)
buf->off = 0; buf->off = 0;
debug_return_bool(true); debug_return_bool(true);
oom:
sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
debug_return_bool(false);
} }
/* /*

View File

@@ -297,23 +297,31 @@ get_free_buf(size_t len, struct connection_closure *closure)
if (buf != NULL) { if (buf != NULL) {
TAILQ_REMOVE(&closure->free_bufs, buf, entries); TAILQ_REMOVE(&closure->free_bufs, buf, entries);
} else { } else {
if ((buf = calloc(1, sizeof(*buf))) == NULL) { if ((buf = calloc(1, sizeof(*buf))) == NULL)
sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); goto oom;
debug_return_ptr(NULL);
}
} }
if (len > buf->size) { if (len > buf->size) {
free(buf->data); const unsigned int new_size = sudo_pow2_roundup(len);
buf->size = sudo_pow2_roundup(len); if (new_size < len) {
if ((buf->data = malloc(buf->size)) == NULL) { /* overflow */
sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); errno = ENOMEM;
free(buf); goto oom;
buf = NULL;
} }
free(buf->data);
if ((buf->data = malloc(new_size)) == NULL)
goto oom;
buf->size = new_size;
} }
debug_return_ptr(buf); 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 static bool

View File

@@ -268,6 +268,11 @@ journal_seek(struct timespec *target, struct connection_closure *closure)
if (msg_len > bufsize) { if (msg_len > bufsize) {
bufsize = sudo_pow2_roundup(msg_len); bufsize = sudo_pow2_roundup(msg_len);
if (bufsize < msg_len) {
/* overflow */
closure->errstr = _("unable to allocate memory");
break;
}
free(buf); free(buf);
if ((buf = malloc(bufsize)) == NULL) { if ((buf = malloc(bufsize)) == NULL) {
closure->errstr = _("unable to allocate memory"); closure->errstr = _("unable to allocate memory");

View File

@@ -256,7 +256,7 @@ get_free_buf(size_t len, struct client_closure *closure)
if (len > buf->size) { if (len > buf->size) {
free(buf->data); free(buf->data);
buf->size = sudo_pow2_roundup(len); 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")); sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
free(buf); free(buf);
buf = NULL; buf = NULL;
@@ -285,13 +285,22 @@ read_io_buf(struct client_closure *closure)
/* Expand buf as needed. */ /* Expand buf as needed. */
if (timing->u.nbytes > closure->bufsize) { if (timing->u.nbytes > closure->bufsize) {
free(closure->buf); const size_t new_size = sudo_pow2_roundup(timing->u.nbytes);
closure->bufsize = sudo_pow2_roundup(timing->u.nbytes); if (new_size < timing->u.nbytes) {
if ((closure->buf = malloc(closure->bufsize)) == NULL) { /* overflow */
errno = ENOMEM;
sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
timing->u.nbytes = 0; timing->u.nbytes = 0;
debug_return_bool(false); debug_return_bool(false);
} }
free(closure->buf);
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, nread = iolog_read(&closure->iolog_files[timing->event], closure->buf,

View File

@@ -732,13 +732,19 @@ fmt_client_message(struct client_closure *closure, ClientMessage *msg)
/* Resize buffer as needed. */ /* Resize buffer as needed. */
if (len > buf->size) { if (len > buf->size) {
free(buf->data); const unsigned int new_size = sudo_pow2_roundup(len);
buf->size = sudo_pow2_roundup(len); if (new_size < len) {
if ((buf->data = malloc(buf->size)) == NULL) { /* overflow */
errno = ENOMEM;
sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
buf->size = 0;
goto done; goto done;
} }
free(buf->data);
if ((buf->data = malloc(new_size)) == NULL) {
sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
goto done;
}
buf->size = new_size;
} }
memcpy(buf->data, &msg_len, sizeof(msg_len)); memcpy(buf->data, &msg_len, sizeof(msg_len));
@@ -1633,16 +1639,19 @@ expand_buf(struct connection_buffer *buf, unsigned int needed)
if (buf->size < needed) { if (buf->size < needed) {
/* Expand buffer. */ /* Expand buffer. */
needed = sudo_pow2_roundup(needed); const unsigned int newsize = sudo_pow2_roundup(needed);
if ((newdata = malloc(needed)) == NULL) { if (newsize < needed) {
sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); /* overflow */
debug_return_bool(false); errno = ENOMEM;
goto oom;
} }
if ((newdata = malloc(needed)) == NULL)
goto oom;
if (buf->off > 0) if (buf->off > 0)
memcpy(newdata, buf->data + buf->off, buf->len - buf->off); memcpy(newdata, buf->data + buf->off, buf->len - buf->off);
free(buf->data); free(buf->data);
buf->data = newdata; buf->data = newdata;
buf->size = needed; buf->size = newsize;
} else { } else {
/* Just reset existing buffer. */ /* Just reset existing buffer. */
if (buf->off > 0) { if (buf->off > 0) {
@@ -1654,6 +1663,9 @@ expand_buf(struct connection_buffer *buf, unsigned int needed)
buf->off = 0; buf->off = 0;
debug_return_bool(true); debug_return_bool(true);
oom:
sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
debug_return_bool(false);
} }
/* /*