]> git.ipfire.org Git - thirdparty/rsync.git/commitdiff
rsync.h: lower MAX_WIRE_DEL_STAT to avoid signed-int overflow in read_del_stats
authorAndrew Tridgell <andrew@tridgell.net>
Thu, 14 May 2026 04:45:21 +0000 (14:45 +1000)
committerAndrew Tridgell <andrew@tridgell.net>
Wed, 20 May 2026 00:01:22 +0000 (10:01 +1000)
read_del_stats() in main.c accumulates 5 wire-supplied counts into
the int32 stats.deleted_files field:

    stats.deleted_files  = read_varint_bounded(..., MAX_WIRE_DEL_STAT, ...);
    stats.deleted_files += stats.deleted_dirs     = ...;
    stats.deleted_files += stats.deleted_symlinks = ...;
    stats.deleted_files += stats.deleted_devices  = ...;
    stats.deleted_files += stats.deleted_specials = ...;

With the previous MAX_WIRE_DEL_STAT = 2^30 (1.07 GB) the worst-case
sum is 5 * 2^30 = 5.37 GB; three maximal values already exceed
INT32_MAX = 2.15 GB on the third "+=", triggering signed integer
overflow (C99 6.5/5 -- undefined behaviour, the compiler may assume
it cannot happen and elide subsequent checks).

The bound was introduced in f0155902 ("defence-in-depth: bound
wire-supplied counts and lengths") with a commit message claiming
"per-summand cap so the total can't overflow", but 2^30 * 5 does
overflow.  Lower the per-summand cap to 2^28 (= 268M) so the worst
case is 5 * 2^28 = 1.34 GB < INT32_MAX with margin.  2^28 deletions
per category is still vastly above any plausible real transfer.

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

diff --git a/rsync.h b/rsync.h
index 4d40542e75633a55f776b095b88b10a0403389d3..cdc2d2c0dfde5d8aadf5b4d2d5a0ff590472845d 100644 (file)
--- a/rsync.h
+++ b/rsync.h
 #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)
+/* MAX_WIRE_DEL_STAT is the per-category cap for read_del_stats() in main.c,
+ * which accumulates 5 wire-supplied counts into the int32 stats.deleted_files
+ * accumulator.  Capped at 2^28 so 5 * 2^28 = 1.34 GB stays under INT32_MAX
+ * (2.15 GB) with margin -- a higher cap (e.g. 2^30) would let a hostile peer
+ * supplying 3+ max-sized counts overflow the accumulator, which is signed-int
+ * UB.  2^28 is still well above any plausible real transfer's deletion count. */
+#define MAX_WIRE_DEL_STAT      ((int32)1 << 28)
 
 #define ROUND_UP_1024(siz) ((siz) & (1024-1) ? ((siz) | (1024-1)) + 1 : (siz))