defence-in-depth: bound wire-supplied counts and lengths

Multiple receiver-side fields read from the wire were trusted
without upper-bound checks. A hostile peer could either request
extreme allocations (DoS via --max-alloc) or, on platforms where
read_varint returned a negative value, push ~SIZE_MAX through the
size_t conversion to wrap downstream length checks.

Introduce read_int_bounded(), read_varint_bounded() and
read_varint_size() in io.c so wire-derived integer ranges are
checked at the read site rather than scattered across each
caller, with RERR_PROTOCOL on out-of-range input.

Apply the bounded primitives to:
  - sum->count (checksum count -- previously could overflow
    (size_t)count * xfer_sum_len on 32-bit with raised max-alloc)
  - xattrs: count, name_len, datum_len, plus rel_pos overflow
    detect to stop chain wrapping the num accumulator
  - acls: ida-entry count
  - flist: file mode S_IFMT validation, modtime_nsec range check
  - delete-stat counters in main: per-summand cap so the total
    can't overflow a signed 32-bit accumulator

Reporters include Joshua Rogers (checksum-count overflow finding).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

diff --git a/acls.c b/acls.c
index 4d67ff4d16f746356312a525325b028e45893f8a..c60a7087f9377aa4f33e29bdcedf8ec7ce31b6de 100644 (file)
--- a/acls.c
+++ b/acls.c
@@ -697,7 +697,7 @@ static uint32 recv_acl_access(int f, uchar *name_follows_ptr)
 static uchar recv_ida_entries(int f, ida_entries *ent)
 {
        uchar computed_mask_bits = 0;
-       int i, count = read_varint(f);
+       int i, count = read_varint_bounded(f, 0, MAX_WIRE_ACL_COUNT, "ACL count");
 
        ent->idas = count ? new_array(id_access, count) : NULL;
        ent->count = count;

diff --git a/flist.c b/flist.c
index 7bf3030e5a91e8f41fb40eed79aebc089b0cdfa9..2ec07f54a4ea95f3d9e0eee80857a034f64a9414 100644 (file)
--- a/flist.c
+++ b/flist.c
@@ -840,9 +840,9 @@ static struct file_struct *recv_file_entry(int f, struct file_list *flist, int x
        }
        if (xflags & XMIT_MOD_NSEC)
 #ifndef CAN_SET_NSEC
-               (void)read_varint(f);
+               (void)read_varint_bounded(f, 0, MAX_WIRE_NSEC, "modtime_nsec");
 #else
-               modtime_nsec = read_varint(f);
+               modtime_nsec = read_varint_bounded(f, 0, MAX_WIRE_NSEC, "modtime_nsec");
        else
                modtime_nsec = 0;
 #endif
@@ -861,8 +861,19 @@ static struct file_struct *recv_file_entry(int f, struct file_list *flist, int x
 #endif
        }
 #endif
-       if (!(xflags & XMIT_SAME_MODE))
+       if (!(xflags & XMIT_SAME_MODE)) {
                mode = from_wire_mode(read_int(f));
+               /* Reject modes whose type bits are not one of the standard
+                * file types; otherwise garbage mode values propagate through
+                * the file-type checks below unpredictably. */
+               if (!S_ISREG(mode) && !S_ISDIR(mode) && !S_ISLNK(mode)
+                && !S_ISCHR(mode) && !S_ISBLK(mode)
+                && !S_ISFIFO(mode) && !S_ISSOCK(mode)) {
+                       rprintf(FERROR, "invalid file mode 0%o for %s [%s]\n",
+                               (unsigned)mode, lastname, who_am_i());
+                       exit_cleanup(RERR_PROTOCOL);
+               }
+       }
        if (atimes_ndx && !S_ISDIR(mode) && !(xflags & XMIT_SAME_ATIME)) {
                atime = read_varlong(f, 4);
 #if SIZEOF_TIME_T < SIZEOF_INT64

diff --git a/io.c b/io.c
index c5c14076e8f2c51c38331f47d9f5e5adfc8150fb..08e7e0aad3d8763e58a4ae75f2207f61e8d503aa 100644 (file)
--- a/io.c
+++ b/io.c
@@ -1868,6 +1868,45 @@ int64 read_varlong(int f, uchar min_bytes)
        return u.x;
 }
 
+/* Read an int32 and verify lo <= v <= hi. On out-of-range, abort with a
+ * protocol error naming "what". The bound is co-located with the read so it
+ * cannot be forgotten by a downstream user. */
+int32 read_int_bounded(int f, int32 lo, int32 hi, const char *what)
+{
+       int32 v = read_int(f);
+       if (v < lo || v > hi) {
+               rprintf(FERROR, "wire value %s out of range: %ld not in [%ld,%ld] [%s]\n",
+                       what, (long)v, (long)lo, (long)hi, who_am_i());
+               exit_cleanup(RERR_PROTOCOL);
+       }
+       return v;
+}
+
+/* As read_int_bounded but for varint-encoded values. */
+int32 read_varint_bounded(int f, int32 lo, int32 hi, const char *what)
+{
+       int32 v = read_varint(f);
+       if (v < lo || v > hi) {
+               rprintf(FERROR, "wire value %s out of range: %ld not in [%ld,%ld] [%s]\n",
+                       what, (long)v, (long)lo, (long)hi, who_am_i());
+               exit_cleanup(RERR_PROTOCOL);
+       }
+       return v;
+}
+
+/* Read a varint that will be used as a size_t. Rejects negative values
+ * (which would wrap to ~SIZE_MAX) and values exceeding the supplied max. */
+size_t read_varint_size(int f, size_t max, const char *what)
+{
+       int32 v = read_varint(f);
+       if (v < 0 || (size_t)v > max) {
+               rprintf(FERROR, "wire size %s out of range: %ld > %lu [%s]\n",
+                       what, (long)v, (unsigned long)max, who_am_i());
+               exit_cleanup(RERR_PROTOCOL);
+       }
+       return (size_t)v;
+}
+
 int64 read_longint(int f)
 {
 #if SIZEOF_INT64 >= 8
@@ -1974,6 +2013,21 @@ void read_sum_head(int f, struct sum_struct *sum)
                        (long)sum->count, who_am_i());
                exit_cleanup(RERR_PROTOCOL);
        }
+       /* Guard against integer overflow in downstream allocations sized by
+        * count*element_size. my_alloc uses divide-not-multiply so it is
+        * already wraparound-safe, but checking here gives a clearer error
+        * and also covers the (size_t)count * xfer_sum_len arithmetic that
+        * is performed *before* reaching my_alloc. */
+       if (xfer_sum_len > 0 && (size_t)sum->count > SIZE_MAX / (size_t)xfer_sum_len) {
+               rprintf(FERROR, "Invalid checksum count %ld (too large) [%s]\n",
+                       (long)sum->count, who_am_i());
+               exit_cleanup(RERR_PROTOCOL);
+       }
+       if ((size_t)sum->count > SIZE_MAX / sizeof(struct sum_buf)) {
+               rprintf(FERROR, "Invalid checksum count %ld (sum_buf overflow) [%s]\n",
+                       (long)sum->count, who_am_i());
+               exit_cleanup(RERR_PROTOCOL);
+       }
        sum->blength = read_int(f);
        if (sum->blength < 0 || sum->blength > max_blength) {
                rprintf(FERROR, "Invalid block length %ld [%s]\n",

diff --git a/main.c b/main.c
index ccad28a1e9554a7be37edb33a729ca915a4f902d..6347934e2e97c8636455afe96dad7fd6703e5573 100644 (file)
--- a/main.c
+++ b/main.c
@@ -239,11 +239,11 @@ void write_del_stats(int f)
 
 void read_del_stats(int f)
 {
-       stats.deleted_files = read_varint(f);
-       stats.deleted_files += stats.deleted_dirs = read_varint(f);
-       stats.deleted_files += stats.deleted_symlinks = read_varint(f);
-       stats.deleted_files += stats.deleted_devices = read_varint(f);
-       stats.deleted_files += stats.deleted_specials = read_varint(f);
+       stats.deleted_files = read_varint_bounded(f, 0, MAX_WIRE_DEL_STAT, "deleted_files");
+       stats.deleted_files += stats.deleted_dirs = read_varint_bounded(f, 0, MAX_WIRE_DEL_STAT, "deleted_dirs");
+       stats.deleted_files += stats.deleted_symlinks = read_varint_bounded(f, 0, MAX_WIRE_DEL_STAT, "deleted_symlinks");
+       stats.deleted_files += stats.deleted_devices = read_varint_bounded(f, 0, MAX_WIRE_DEL_STAT, "deleted_devices");
+       stats.deleted_files += stats.deleted_specials = read_varint_bounded(f, 0, MAX_WIRE_DEL_STAT, "deleted_specials");
 }
 
 static void become_copy_as_user()

diff --git a/rsync.h b/rsync.h
index 479ac4848991401cf8df4597283d23fdd9c9df32..4d40542e75633a55f776b095b88b10a0403389d3 100644 (file)
--- a/rsync.h
+++ b/rsync.h
@@ -163,6 +163,23 @@
 /* For compatibility with older rsyncs */
 #define OLD_MAX_BLOCK_SIZE ((int32)1 << 29)
 
+/* Policy ceilings on attacker-controlled wire values. Picked well above any
+ * legitimate filesystem / protocol traffic but well below sizes that could
+ * cause integer overflow or DoS-grade allocations. See input_checking.txt.
+ *
+ * Note on MAX_WIRE_XATTR_DATALEN: xattr datum size is bounded only by the
+ * wire-format maximum (signed int32 varint, ~2GB). macOS resource forks
+ * are transferred as the com.apple.ResourceFork xattr and can legitimately
+ * be many GB; --max-alloc (default 1GB, configurable) is the real
+ * allocation cap. read_varint_size() still rejects negative values so a
+ * hostile peer cannot wrap to ~SIZE_MAX. */
+#define MAX_WIRE_XATTR_COUNT   65536
+#define MAX_WIRE_XATTR_NAMELEN 4096
+#define MAX_WIRE_XATTR_DATALEN ((int32)0x7fffffff)
+#define MAX_WIRE_ACL_COUNT     65536
+#define MAX_WIRE_NSEC          999999999
+#define MAX_WIRE_DEL_STAT      ((int32)1 << 30)
+
 #define ROUND_UP_1024(siz) ((siz) & (1024-1) ? ((siz) | (1024-1)) + 1 : (siz))
 
 #define IOERR_GENERAL  (1<<0) /* For backward compatibility, this must == 1 */

diff --git a/xattrs.c b/xattrs.c
index 5f740bb53afac036650dfed8ce99968d519bb165..99795f244bbb5ef2940ecced165240e626ec4832 100644 (file)
--- a/xattrs.c
+++ b/xattrs.c
@@ -697,6 +697,13 @@ int recv_xattr_request(struct file_struct *file, int f_in)
        rxa = lst->items;
        num = 0;
        while ((rel_pos = read_varint(f_in)) != 0) {
+               /* Detect signed overflow before the accumulating add. A hostile
+                * peer could otherwise wrap 'num' to land on an arbitrary value. */
+               if ((rel_pos > 0 && num > INT_MAX - rel_pos)
+                || (rel_pos < 0 && num < INT_MIN - rel_pos)) {
+                       rprintf(FERROR, "xattr rel_pos accumulation overflow [%s]\n", who_am_i());
+                       exit_cleanup(RERR_PROTOCOL);
+               }
                num += rel_pos;
                if (am_sender) {
                        /* The sender-related num values are only in order on the sender.
@@ -742,7 +749,7 @@ int recv_xattr_request(struct file_struct *file, int f_in)
                }
 
                old_datum = rxa->datum;
-               rxa->datum_len = read_varint(f_in);
+               rxa->datum_len = read_varint_size(f_in, MAX_WIRE_XATTR_DATALEN, "xattr datum_len");
 
                if (SIZE_MAX - rxa->name_len < rxa->datum_len)
                        overflow_exit("recv_xattr_request");
@@ -783,7 +790,8 @@ void receive_xattr(int f, struct file_struct *file)
                return;
        }
 
-       if ((count = read_varint(f)) != 0) {
+       count = read_varint_bounded(f, 0, MAX_WIRE_XATTR_COUNT, "xattr count");
+       if (count != 0) {
                (void)EXPAND_ITEM_LIST(&temp_xattr, rsync_xa, count);
                temp_xattr.count = 0;
        }
@@ -791,8 +799,8 @@ void receive_xattr(int f, struct file_struct *file)
        for (num = 1; num <= count; num++) {
                char *ptr, *name;
                rsync_xa *rxa;
-               size_t name_len = read_varint(f);
-               size_t datum_len = read_varint(f);
+               size_t name_len = read_varint_size(f, MAX_WIRE_XATTR_NAMELEN, "xattr name_len");
+               size_t datum_len = read_varint_size(f, MAX_WIRE_XATTR_DATALEN, "xattr datum_len");
                size_t dget_len = datum_len > MAX_FULL_DATUM ? 1 + (size_t)xattr_sum_len : datum_len;
                size_t extra_len = MIGHT_NEED_RPRE ? RPRE_LEN : 0;
                if (SIZE_MAX - dget_len < extra_len || SIZE_MAX - dget_len - extra_len < name_len)