From 13d023e617933c6d6a8cfd802a0b5cc2cb0dea67 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Fri, 26 Jun 2026 18:53:37 +1000 Subject: [PATCH] hashtable, flist: fix integer overflows in size computations hashtable_create() and the grow path computed the slot-array byte count as new_array0(char, size * node_size) in 32-bit int arithmetic; for a large peer/data-driven size the product wrapped to a tiny value, bypassing my_alloc's --max-alloc guard (which only saw the already-wrapped count), so the table was under-allocated while tbl->size kept the huge size -- a later node access then ran out of bounds (heap overflow; ASan-confirmed). Pass size and node_size as SEPARATE factors so my_alloc checks both before multiplying; guard each *2 doubling against int overflow BEFORE it happens; make HASH_LOAD_LIMIT divide before multiplying; promote the HT_NODE index multiply to size_t; and test size < 16 first so a negative req short-circuits the size-1 (INT_MIN UB). flist_expand()'s int growth math (used+extra and *=4 / *=2 / += FLIST_LINEAR) could overflow past INT_MAX on a very large file list; guard each operation before it overflows and refuse rather than under-size the realloc. (Leonid Bugaev May-2026 re-audit, KI-11/12/13.) --- flist.c | 25 ++++++++++++++++++++++--- hashtable.c | 31 +++++++++++++++++++++++-------- rsync.h | 2 +- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/flist.c b/flist.c index f0d869c0..45521d85 100644 --- a/flist.c +++ b/flist.c @@ -367,17 +367,31 @@ static void flist_expand(struct file_list *flist, int extra) { struct file_struct **new_ptr; + /* Refuse BEFORE any int arithmetic below can overflow: used+extra (computed + * in the early-return and the cap below) and the malloced growth math. Only + * reachable past INT_MAX entries (my_alloc's --max-alloc cap normally stops + * the list growing anywhere near there). */ + if (extra < 0 || flist->used < 0 || flist->used > INT_MAX - extra) + goto too_large; + if (flist->used + extra <= flist->malloced) return; if (flist->malloced < FLIST_START) flist->malloced = FLIST_START; - else if (flist->malloced >= FLIST_LINEAR) + else if (flist->malloced >= FLIST_LINEAR) { + if (flist->malloced > INT_MAX - FLIST_LINEAR) + goto too_large; flist->malloced += FLIST_LINEAR; - else if (flist->malloced < FLIST_START_LARGE/16) + } else if (flist->malloced < FLIST_START_LARGE/16) { + if (flist->malloced > INT_MAX/4) + goto too_large; flist->malloced *= 4; - else + } else { + if (flist->malloced > INT_MAX/2) + goto too_large; flist->malloced *= 2; + } /* In case count jumped or we are starting the list * with a known size just set it. */ @@ -394,6 +408,11 @@ static void flist_expand(struct file_list *flist, int extra) } flist->files = new_ptr; + return; + + too_large: + rprintf(FERROR, "[%s] file list has grown too large to expand\n", who_am_i()); + exit_cleanup(RERR_MALLOC); } static void flist_done_allocating(struct file_list *flist) diff --git a/hashtable.c b/hashtable.c index 2cc4e550..dfecf42d 100644 --- a/hashtable.c +++ b/hashtable.c @@ -19,7 +19,7 @@ #include "rsync.h" -#define HASH_LOAD_LIMIT(size) ((size)*3/4) +#define HASH_LOAD_LIMIT(size) ((size)/4*3) /* /4 first: never overflows int */ struct hashtable *hashtable_create(int size, int key64) { @@ -28,15 +28,25 @@ struct hashtable *hashtable_create(int size, int key64) int node_size = key64 ? sizeof (struct ht_int64_node) : sizeof (struct ht_int32_node); - /* Pick a power of 2 that can hold the requested size. */ - if (size & (size-1) || size < 16) { + /* Pick a power of 2 that can hold the requested size. Test size < 16 first + * so a negative/zero req short-circuits before the size-1 (INT_MIN is UB). */ + if (size < 16 || (size & (size-1))) { size = 16; - while (size < req) + while (size < req) { + if (size > INT_MAX/2) { /* the next doubling would overflow int */ + rprintf(FERROR, "[%s] hashtable_create: requested size %d is too large\n", + who_am_i(), req); + exit_cleanup(RERR_MALLOC); + } size *= 2; + } } tbl = new(struct hashtable); - tbl->nodes = new_array0(char, size * node_size); + /* Pass size and node_size as SEPARATE factors so my_alloc's overflow / + * --max-alloc guard sees both; computing size*node_size as int would wrap to + * a tiny count and under-allocate (heap overflow on later node access). */ + tbl->nodes = my_alloc(do_calloc, size, node_size, __FILE__, __LINE__); tbl->size = size; tbl->entries = 0; tbl->node_size = node_size; @@ -90,10 +100,15 @@ void *hashtable_find(struct hashtable *tbl, int64 key, void *data_when_new) if (data_when_new && tbl->entries > HASH_LOAD_LIMIT(tbl->size)) { void *old_nodes = tbl->nodes; - int size = tbl->size * 2; - int i; + int size, i; - tbl->nodes = new_array0(char, size * tbl->node_size); + if (tbl->size > INT_MAX/2) { /* doubling would overflow int */ + rprintf(FERROR, "[%s] hashtable grow: size overflow\n", who_am_i()); + exit_cleanup(RERR_MALLOC); + } + size = tbl->size * 2; + /* Separate factors so my_alloc's guard sees both (see hashtable_create). */ + tbl->nodes = my_alloc(do_calloc, size, tbl->node_size, __FILE__, __LINE__); tbl->size = size; tbl->entries = 0; diff --git a/rsync.h b/rsync.h index 6c8ba4c9..de36ee3d 100644 --- a/rsync.h +++ b/rsync.h @@ -728,7 +728,7 @@ struct ht_int64_node { int64 key; }; -#define HT_NODE(tbl, bkts, i) ((void*)((char*)(bkts) + (i)*(tbl)->node_size)) +#define HT_NODE(tbl, bkts, i) ((void*)((char*)(bkts) + (size_t)(i)*(tbl)->node_size)) #define HT_KEY(node, k64) ((k64)? ((struct ht_int64_node*)(node))->key \ : (int64)((struct ht_int32_node*)(node))->key)