Widen size checks instead of truncating to size_t

.gitignore

@@ -239,6 +239,7 @@
 /tests/lib/pread.t
 /tests/lib/pwrite.t
 /tests/lib/qio.t
+/tests/lib/readin.t
 /tests/lib/reallocarray.t
 /tests/lib/reservedfd.t
 /tests/lib/setenv.t
@@ -261,9 +262,11 @@
 /tests/overview/ovsqlite.t
 /tests/overview/ovsqlite-read.t
 /tests/overview/ovsqlite-write.t
+/tests/overview/tdx-group.t
 /tests/overview/tradindexed.t
 /tests/overview/xref.t
 /tests/perl/minimum-version.t
+/tests/storage/caf.t
 /tests/storage/cancel-tombstone.t
 /tests/util/innbind.t
 **/.libs/

MANIFEST

@@ -1013,6 +1013,7 @@ tests/lib/network/server-t.c          Tests for lib/network.c (server-oriented)
 tests/lib/pread-t.c                   Tests for lib/pread.c
 tests/lib/pwrite-t.c                  Tests for lib/pwrite.c
 tests/lib/qio-t.c                     Tests for lib/qio.c
+tests/lib/readin-t.c                  Tests for lib/readin.c
 tests/lib/reallocarray-t.c            Tests for lib/reallocarray.c
 tests/lib/reservedfd-t.c              Tests for reserved file descriptors
 tests/lib/setenv-t.c                  Tests for lib/setenv.c
@@ -1037,12 +1038,14 @@ tests/overview/ovsqlite-integ.t       Integration test for ovsqlite direct reade
 tests/overview/ovsqlite-read-t.c      Direct reader verification for integration test
 tests/overview/ovsqlite-t.c           Unit tests for ovsqlite direct reader
 tests/overview/ovsqlite-write-t.c     Writer helper for ovsqlite integration test
+tests/overview/tdx-group-t.c          Tests for tradindexed group index sizes
 tests/overview/xref-t.c               Test storing overview data by Xref
 tests/perl                            Test suite for Perl scripts (Directory)
 tests/perl/minimum-version.t.in       Tests for not too-new features of Perl
 tests/runtests.c                      The test suite driver program
 tests/storage                         Test suite for storage (Directory)
 tests/storage/archive.t               Tests for backends/archive
+tests/storage/caf-t.c                 Tests for CAF file validation and cleaning
 tests/storage/cancel-tombstone-t.c    Tests for SMcanceltombstone
 tests/storage/makehistory.t           Tests for expire/makehistory
 tests/storage/sm.t                    Tests for frontends/sm

backends/innxmit.c

@@ -1062,14 +1062,21 @@ article_open(const char *path, const char *id)
             return NULL;
         if (fstat(fd, &st) < 0) {
             syswarn("requeue %s", path);
+            close(fd);
+            Requeue(path, id);
+            return NULL;
+        }
+        if (st.st_size < 0 || (uintmax_t) st.st_size > SIZE_MAX) {
+            warn("requeue %s: article is too large", path);
+            close(fd);
             Requeue(path, id);
             return NULL;
         }
         article = xmalloc(sizeof(ARTHANDLE));
         article->type = TOKEN_EMPTY;
-        article->len = st.st_size;
+        article->len = (size_t) st.st_size;
         data = xmalloc(article->len);
-        if (xread(fd, data, article->len) < 0) {
+        if (xread(fd, data, st.st_size) < 0) {
             syswarn("requeue %s", path);
             free(data);
             free(article);
@@ -1088,6 +1095,13 @@ article_open(const char *path, const char *id)
         }
         if (p[-1] != '\r') {
             p = wire_from_native(data, article->len, &length);
+            if (p == NULL) {
+                syswarn("requeue %s: cannot convert article", path);
+                free(data);
+                free(article);
+                Requeue(path, id);
+                return NULL;
+            }
             free(data);
             data = p;
             article->len = length;

frontends/rnews.c

@@ -232,6 +232,11 @@ Process(char *article, size_t artlen)
 
     /* Convert the article to wire format. */
     wirefmt = wire_from_native(article, artlen, &length);
+    if (wirefmt == NULL) {
+        Reject(article, artlen, "bad_article %s",
+               "too large after wire conversion");
+        return true;
+    }
 
     /* Make sure that all the headers are there, note the ID. */
     for (hp = RequiredHeaders; hp < ARRAY_END(RequiredHeaders); hp++) {

frontends/sm.c

@@ -148,6 +148,8 @@ store_article(int fd)
         sysdie("cannot read article");
     }
     text = wire_from_native(article->data, article->left, &size);
+    if (text == NULL)
+        sysdie("cannot convert article to wire format");
     buffer_free(article);
     result = store_article_common(text, size);
     free(text);

include/inn/wire.h

@@ -46,7 +46,8 @@ char *wire_endheader(const char *header, const char *end);
 
 /* Given an article and length in non-wire format, return a malloced region
    containing the article in wire format and set newlen to the length of the
-   new article. */
+   new article.  Returns NULL with errno set to EOVERFLOW if the converted
+   article would be too large to represent. */
 char *wire_from_native(const char *article, size_t len, size_t *newlen);
 
 /* Given an article and length in wire format, return a malloced region

innd/icd.c

@@ -454,7 +454,13 @@ ICDreadactive(char **endp)
                ICDactpath);
         exit(1);
     }
-    ICDactsize = Sb.st_size;
+    if (Sb.st_size < 0 || (uintmax_t) Sb.st_size > INT_MAX) {
+        syslog(L_FATAL, "%s active file %s is too large", LogName,
+               ICDactpath);
+        exit(1);
+    }
+    ICDactsize = (int) Sb.st_size;
+
     ICDactpointer = mmap(NULL, ICDactsize, PROT_READ | PROT_WRITE, MAP_SHARED,
                          ICDactfd, 0);
     if (ICDactpointer == (char *) -1) {
@@ -465,11 +471,31 @@ ICDreadactive(char **endp)
 
 #else /* !HAVE_MMAP */
 
+    /* Reject an already-oversized active file before asking the generic
+       reader to allocate it.  ICDactsize below still comes from the exact
+       stat used to size and read the returned buffer. */
+    if (fstat(ICDactfd, &Sb) < 0) {
+        syslog(L_FATAL, "%s cant fstat %d %s %m", LogName, ICDactfd,
+               ICDactpath);
+        exit(1);
+    }
+    if (Sb.st_size < 0 || (uintmax_t) Sb.st_size > INT_MAX) {
+        syslog(L_FATAL, "%s active file %s is too large", LogName,
+               ICDactpath);
+        exit(1);
+    }
     if ((ICDactpointer = ReadInDescriptor(ICDactfd, &Sb)) == NULL) {
         syslog(L_FATAL, "%s cant read %s %m", LogName, ICDactpath);
         exit(1);
     }
-    ICDactsize = Sb.st_size;
+    if (Sb.st_size < 0 || (uintmax_t) Sb.st_size > INT_MAX) {
+        free(ICDactpointer);
+        ICDactpointer = NULL;
+        syslog(L_FATAL, "%s active file %s is too large", LogName,
+               ICDactpath);
+        exit(1);
+    }
+    ICDactsize = (int) Sb.st_size;
 
 #endif /* HAVE_MMAP */
 

lib/buffer.c

@@ -319,6 +319,11 @@ buffer_read_file(struct buffer *buffer, int fd)
 
     if (fstat(fd, &st) < 0)
         return false;
-    buffer_resize(buffer, st.st_size + used);
+    if (st.st_size < 0 || used > SIZE_MAX - 1023
+        || (uintmax_t) st.st_size > SIZE_MAX - used - 1023) {
+        errno = EOVERFLOW;
+        return false;
+    }
+    buffer_resize(buffer, (size_t) st.st_size + used);
     return buffer_read_all(buffer, fd);
 }

lib/readin.c

@@ -14,14 +14,26 @@
 int
 xread(int fd, char *p, off_t i)
 {
-    int count;
+    ssize_t count;
+    size_t request;
+
+    if (i < 0) {
+        errno = EINVAL;
+        return -1;
+    }
 
     for (; i; p += count, i -= count) {
+        request = (uintmax_t) i > (uintmax_t) SSIZE_MAX ? SSIZE_MAX
+                                                        : (size_t) i;
         do {
-            count = read(fd, p, i);
+            count = read(fd, p, request);
         } while (count == -1 && errno == EINTR);
-        if (count <= 0)
+        if (count < 0)
             return -1;
+        if (count == 0) {
+            errno = EIO;
+            return -1;
+        }
     }
     return 0;
 }
@@ -35,31 +47,32 @@ ReadInDescriptor(int fd, struct stat *Sbp)
 {
     struct stat mystat;
     char *p;
+    size_t size;
     int oerrno;
 
     if (Sbp == NULL)
         Sbp = &mystat;
 
     /* Get the size, and enough memory. */
-    if (fstat(fd, Sbp) < 0) {
-        oerrno = errno;
-        close(fd);
-        errno = oerrno;
+    if (fstat(fd, Sbp) < 0)
+        return NULL;
+    if (Sbp->st_size < 0 || (uintmax_t) Sbp->st_size >= SIZE_MAX) {
+        errno = EOVERFLOW;
         return NULL;
     }
-    p = xmalloc(Sbp->st_size + 1);
+    size = (size_t) Sbp->st_size;
+    p = xmalloc(size + 1);
 
     /* Slurp, slurp. */
     if (xread(fd, p, Sbp->st_size) < 0) {
         oerrno = errno;
         free(p);
-        close(fd);
         errno = oerrno;
         return NULL;
     }
 
     /* Terminate the string; terminate the routine. */
-    p[Sbp->st_size] = '\0';
+    p[size] = '\0';
     return p;
 }
 
@@ -72,12 +85,18 @@ char *
 ReadInFile(const char *name, struct stat *Sbp)
 {
     char *p;
-    int fd;
+    int fd, oerrno;
 
     if ((fd = open(name, O_RDONLY)) < 0)
         return NULL;
 
     p = ReadInDescriptor(fd, Sbp);
-    close(fd);
+    if (p == NULL) {
+        oerrno = errno;
+        close(fd);
+        errno = oerrno;
+    } else {
+        close(fd);
+    }
     return p;
 }

lib/wire.c

@@ -23,6 +23,7 @@
 #include "portable/system.h"
 
 #include <assert.h>
+#include <errno.h>
 
 #include "inn/libinn.h"
 #include "inn/wire.h"
@@ -195,23 +196,36 @@ char *
 wire_from_native(const char *article, size_t len, size_t *newlen)
 {
     size_t bytes;
+    size_t extra;
+    size_t extra_limit;
     char *newart;
     const char *p;
     char *dest;
     bool at_start = true;
 
+    *newlen = 0;
+    if (len > SIZE_MAX - 4) {
+        errno = EOVERFLOW;
+        return NULL;
+    }
+    extra_limit = SIZE_MAX - 4 - len;
+
     /* First go thru article and count number of bytes we need.  Add a CR for
        every LF and an extra character for any period at the beginning of a
-       line for dot-stuffing.  Add 3 characters at the end for .\r\n. */
-    for (bytes = 0, p = article; p < article + len; p++) {
+       line for dot-stuffing.  Each input byte adds at most one extra byte,
+       so extra cannot overflow.  Add 3 characters at the end for .\r\n. */
+    for (extra = 0, p = article; p < article + len; p++) {
         if (at_start && *p == '.')
-            bytes++;
-        bytes++;
+            extra++;
         at_start = (*p == '\n');
         if (at_start)
-            bytes++;
+            extra++;
+    }
+    if (extra > extra_limit) {
+        errno = EOVERFLOW;
+        return NULL;
     }
-    bytes += 3;
+    bytes = len + extra + 3;
 
     /* Now copy the article, making the required changes. */
     newart = xmalloc(bytes + 1);