Hi, supplying large sizes to ibuf functions can lead to integer overflows which are not properly handled.
I have added regression tests as well, which make it easier to see the effects and how to trigger these issues. The ibuf_open adjustment for malloc(0) has been taken from imsg.c as can be seen in the commit history: https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libutil/imsg.c.diff?r1=1.9&r2=1.10&f=h Okay? Comments? Tobias Index: lib/libutil/imsg-buffer.c =================================================================== RCS file: /cvs/src/lib/libutil/imsg-buffer.c,v retrieving revision 1.13 diff -u -p -u -p -r1.13 imsg-buffer.c --- lib/libutil/imsg-buffer.c 31 Mar 2021 17:42:24 -0000 1.13 +++ lib/libutil/imsg-buffer.c 19 Apr 2022 13:04:00 -0000 @@ -40,7 +40,9 @@ ibuf_open(size_t len) if ((buf = calloc(1, sizeof(struct ibuf))) == NULL) return (NULL); - if ((buf->buf = malloc(len)) == NULL) { + if (len == 0) + buf->buf = NULL; + else if ((buf->buf = malloc(len)) == NULL) { free(buf); return (NULL); } @@ -73,7 +75,7 @@ ibuf_realloc(struct ibuf *buf, size_t le unsigned char *b; /* on static buffers max is eq size and so the following fails */ - if (buf->wpos + len > buf->max) { + if (len > SIZE_MAX - buf->wpos || buf->wpos + len > buf->max) { errno = ERANGE; return (-1); } @@ -90,6 +92,11 @@ ibuf_realloc(struct ibuf *buf, size_t le int ibuf_add(struct ibuf *buf, const void *data, size_t len) { + if (len > SIZE_MAX - buf->wpos) { + errno = ERANGE; + return (-1); + } + if (buf->wpos + len > buf->size) if (ibuf_realloc(buf, len) == -1) return (-1); @@ -104,6 +111,11 @@ ibuf_reserve(struct ibuf *buf, size_t le { void *b; + if (len > SIZE_MAX - buf->wpos) { + errno = ERANGE; + return (NULL); + } + if (buf->wpos + len > buf->size) if (ibuf_realloc(buf, len) == -1) return (NULL); @@ -117,7 +129,7 @@ void * ibuf_seek(struct ibuf *buf, size_t pos, size_t len) { /* only allowed to seek in already written parts */ - if (pos + len > buf->wpos) + if (len > SIZE_MAX - pos || pos + len > buf->wpos) return (NULL); return (buf->buf + pos); @@ -202,7 +214,7 @@ msgbuf_drain(struct msgbuf *msgbuf, size for (buf = TAILQ_FIRST(&msgbuf->bufs); buf != NULL && n > 0; buf = next) { next = TAILQ_NEXT(buf, entry); - if (buf->rpos + n >= buf->wpos) { + if (n >= buf->wpos - buf->rpos) { n -= buf->wpos - buf->rpos; ibuf_dequeue(msgbuf, buf); } else { Index: regress/lib/libutil/Makefile =================================================================== RCS file: /cvs/src/regress/lib/libutil/Makefile,v retrieving revision 1.4 diff -u -p -u -p -r1.4 Makefile --- regress/lib/libutil/Makefile 13 May 2019 10:00:29 -0000 1.4 +++ regress/lib/libutil/Makefile 19 Apr 2022 13:04:00 -0000 @@ -2,7 +2,7 @@ # Makefile for regress/libutil -SUBDIR= bcrypt_pbkdf ber fmt_scaled pkcs5_pbkdf2 +SUBDIR= bcrypt_pbkdf ber fmt_scaled imsg pkcs5_pbkdf2 include: Index: regress/lib/libutil/imsg/Makefile =================================================================== RCS file: regress/lib/libutil/imsg/Makefile diff -N regress/lib/libutil/imsg/Makefile --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ regress/lib/libutil/imsg/Makefile 19 Apr 2022 13:04:00 -0000 @@ -0,0 +1,12 @@ +# $OpenBSD$ + +REGRESS_TARGETS= run-regress + +LDFLAGS= -lutil + +CLEANFILES= ibuf_test ibuf_test.d + +run-regress: ibuf_test + ${.OBJDIR}/ibuf_test + +.include <bsd.regress.mk> Index: regress/lib/libutil/imsg/ibuf_test.c =================================================================== RCS file: regress/lib/libutil/imsg/ibuf_test.c diff -N regress/lib/libutil/imsg/ibuf_test.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ regress/lib/libutil/imsg/ibuf_test.c 19 Apr 2022 13:04:00 -0000 @@ -0,0 +1,172 @@ +/* $OpenBSD$ +*/ +/* + * Copyright (c) Tobias Stoeckmann <[email protected]> + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include <sys/queue.h> +#include <sys/types.h> +#include <sys/uio.h> + +#include <imsg.h> +#include <limits.h> +#include <stdint.h> +#include <stdio.h> + +int +test_ibuf_open(void) { + struct ibuf *buf; + + if ((buf = ibuf_open(0)) == NULL) + return 1; + + ibuf_free(buf); + return 0; +} + +int +test_ibuf_dynamic(void) { + struct ibuf *buf; + + if (ibuf_dynamic(100, 0) != NULL) + return 1; + + if ((buf = ibuf_dynamic(10, SIZE_MAX)) == NULL) + return 1; + + ibuf_free(buf); + return 0; +} + +int +test_ibuf_reserve(void) { + struct ibuf *buf; + int ret; + + if ((buf = ibuf_dynamic(10, SIZE_MAX)) == NULL) { + return 1; + } + + if (ibuf_reserve(buf, SIZE_MAX) != NULL) { + ibuf_free(buf); + return 1; + } + + if (ibuf_reserve(buf, 10) == NULL) { + ibuf_free(buf); + return 1; + } + + ret = (ibuf_reserve(buf, SIZE_MAX) != NULL); + + ibuf_free(buf); + return ret; +} + +int +test_ibuf_seek(void) { + struct ibuf *buf; + int ret; + + if ((buf = ibuf_open(10)) == NULL) + return 1; + + ret = (ibuf_seek(buf, 1, SIZE_MAX) != NULL); + + ibuf_free(buf); + return ret; +} + +int +test_msgbuf_drain(void) { + struct msgbuf msgbuf; + struct ibuf *buf; + + msgbuf_init(&msgbuf); + + if ((buf = ibuf_open(4)) == NULL) + return 1; + if (ibuf_add(buf, "test", 4) != 0) { + ibuf_free(buf); + return 1; + } + ibuf_close(&msgbuf, buf); + + if (msgbuf.queued != 1) { + ibuf_free(buf); + return 1; + } + + msgbuf_drain(&msgbuf, 1); + + if (msgbuf.queued != 1) { + ibuf_free(buf); + return 1; + } + + msgbuf_drain(&msgbuf, SIZE_MAX); + + if (msgbuf.queued != 0) { + ibuf_free(buf); + return 1; + } + + return 0; +} + +int +main(void) +{ + extern char *__progname; + + int ret = 0; + + if (test_ibuf_open() != 0) { + printf("FAILED: test_ibuf_open\n"); + ret = 1; + } else + printf("SUCCESS: test_ibuf_open\n"); + + if (test_ibuf_dynamic() != 0) { + printf("FAILED: test_ibuf_dynamic\n"); + ret = 1; + } else + printf("SUCCESS: test_ibuf_dynamic\n"); + + if (test_ibuf_reserve() != 0) { + printf("FAILED: test_ibuf_reserve\n"); + ret = 1; + } else + printf("SUCCESS: test_ibuf_reserve\n"); + + if (test_ibuf_seek() != 0) { + printf("FAILED: test_ibuf_seek\n"); + ret = 1; + } else + printf("SUCCESS: test_ibuf_seek\n"); + + if (test_msgbuf_drain() != 0) { + printf("FAILED: test_msgbuf_drain\n"); + ret = 1; + } else + printf("SUCCESS: test_msgbuf_drain\n"); + + if (ret != 0) { + printf("FAILED: %s\n", __progname); + return 1; + } + + return 0; +}
