From: Phillip Wood Date: Thu, 13 Jul 2023 08:21:55 GMT Subject: Re: [PATCH 02/20] packfile.c: prevent overflow in `load_idx()` Message-ID: <5d2cf09f-34c7-9a88-bab2-8bf348dd13bb@gmail.com> In-Reply-To: Hi Taylor On 13/07/2023 00:37, Taylor Blau wrote: > Prevent an overflow when locating a pack's CRC offset when the number > of packed items is greater than 2^32-1/hashsz by guarding the > computation with an `st_mult()`. > > Signed-off-by: Taylor Blau > --- > packfile.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/packfile.c b/packfile.c > index 89220f0e03..70acf1694b 100644 > --- a/packfile.c > +++ b/packfile.c > @@ -186,7 +186,7 @@ int load_idx(const char *path, const unsigned int hashsz, void *idx_map, > */ > (sizeof(off_t) <= 4)) > return error("pack too large for current definition of off_t in %s", path); > - p->crc_offset = 8 + 4 * 256 + nr * hashsz; > + p->crc_offset = st_add(8 + 4 * 256, st_mult(nr, hashsz)); p->crc_offset is a uint32_t so we're still prone to truncation here unless we change the crc_offset member of struct packed_git to be a size_t. I haven't checked if the other users of crc_offset would need adjusting if we change its type. Best Wishes Phillip > } > > p->index_version = version;