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

diff --git a/lib/libutil/imsg-buffer.c b/lib/libutil/imsg-buffer.c
index 4526842..7abea4e 100644 (file)
--- 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 <henning@openbsd.org>
@@ -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 7641be2..762d36c 100644 (file)
--- 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 (file)
index 0000000..e0a20ef
--- /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 <bsd.regress.mk>

diff --git a/regress/lib/libutil/imsg/ibuf_test.c b/regress/lib/libutil/imsg/ibuf_test.c
new file mode 100644 (file)
index 0000000..a85b31a
--- /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 <tobias@openbsd.org>
+ *
+ * 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;
+}