From c1a45aed656e7d5627c30c92421893a76f370ccb Mon Sep 17 00:00:00 2001 From: tobias Date: Sat, 23 Apr 2022 08:57:52 +0000 Subject: [PATCH] Verify sizes before arithmetic operations Unsigned overflows are not a bug in C but we have to make sure that requested buffer sizes will be actually available. If not, set errno to ERANGE and return an error value. ok deraadt, millert --- lib/libutil/imsg-buffer.c | 18 ++- regress/lib/libutil/Makefile | 4 +- regress/lib/libutil/imsg/Makefile | 12 ++ regress/lib/libutil/imsg/ibuf_test.c | 172 +++++++++++++++++++++++++++ 4 files changed, 200 insertions(+), 6 deletions(-) create mode 100644 regress/lib/libutil/imsg/Makefile create mode 100644 regress/lib/libutil/imsg/ibuf_test.c diff --git a/lib/libutil/imsg-buffer.c b/lib/libutil/imsg-buffer.c index 4526842bd86..7abea4e0deb 100644 --- a/lib/libutil/imsg-buffer.c +++ b/lib/libutil/imsg-buffer.c @@ -1,4 +1,4 @@ -/* $OpenBSD: imsg-buffer.c,v 1.13 2021/03/31 17:42:24 eric Exp $ */ +/* $OpenBSD: imsg-buffer.c,v 1.14 2022/04/23 08:57:52 tobias Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -73,7 +73,7 @@ ibuf_realloc(struct ibuf *buf, size_t len) 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 +90,11 @@ ibuf_realloc(struct ibuf *buf, size_t len) 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 +109,11 @@ ibuf_reserve(struct ibuf *buf, size_t len) { 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 +127,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 +212,7 @@ msgbuf_drain(struct msgbuf *msgbuf, size_t n) 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 { diff --git a/regress/lib/libutil/Makefile b/regress/lib/libutil/Makefile index 7641be2de0e..762d36cd606 100644 --- a/regress/lib/libutil/Makefile +++ b/regress/lib/libutil/Makefile @@ -1,8 +1,8 @@ -# $OpenBSD: Makefile,v 1.4 2019/05/13 10:00:29 rob Exp $ +# $OpenBSD: Makefile,v 1.5 2022/04/23 08:57:52 tobias Exp $ # Makefile for regress/libutil -SUBDIR= bcrypt_pbkdf ber fmt_scaled pkcs5_pbkdf2 +SUBDIR= bcrypt_pbkdf ber fmt_scaled imsg pkcs5_pbkdf2 include: diff --git a/regress/lib/libutil/imsg/Makefile b/regress/lib/libutil/imsg/Makefile new file mode 100644 index 00000000000..e0a20efa6e1 --- /dev/null +++ b/regress/lib/libutil/imsg/Makefile @@ -0,0 +1,12 @@ +# $OpenBSD: Makefile,v 1.1 2022/04/23 08:57:52 tobias Exp $ + +REGRESS_TARGETS= run-regress + +LDFLAGS= -lutil + +CLEANFILES= ibuf_test ibuf_test.d + +run-regress: ibuf_test + ${.OBJDIR}/ibuf_test + +.include diff --git a/regress/lib/libutil/imsg/ibuf_test.c b/regress/lib/libutil/imsg/ibuf_test.c new file mode 100644 index 00000000000..a85b31a4d85 --- /dev/null +++ b/regress/lib/libutil/imsg/ibuf_test.c @@ -0,0 +1,172 @@ +/* $OpenBSD: ibuf_test.c,v 1.1 2022/04/23 08:57:52 tobias Exp $ +*/ +/* + * Copyright (c) Tobias Stoeckmann + * + * 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 +#include +#include + +#include +#include +#include +#include + +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; +} -- 2.20.1