From: Srujith A Poojary <poojarysrujith@gmail.com>
In create_delta_index(), buffer sizes for allocating the delta index are calculated using arithmetic operations on entries and hsize:
memsize = sizeof(*hash) * hsize + sizeof(*entry) * entries
To prevent potential size_t wrap-around when computing allocation sizes on 32-bit systems, replace manual arithmetic with overflow-checked helpers (st_mult, st_add, st_add3).
For the unpacked index allocation:
- sizeof(struct unpacked_index_entry) is 12 bytes on 32-bit systems,
which may be padded to 16 bytes depending on compiler alignment rules.
- sizeof(struct index_entry *) is 4 bytes.
- entries is capped by (0xffffffu / RABIN_WINDOW) where RABIN_WINDOW is 16,
limiting entries to 1,048,575.
- hsize is the smallest power of 2 greater than or equal to entries,
capped at 1,048,576.
While entries is bounded in current code, using st_mult() and st_add() guarantees that any unexpected growth in struct sizes or bounds changes will safely die() on size_t overflow instead of wrapping around.
For the packed index allocation:
- Passing (hsize + 1) as an argument to st_add3() is safe from overflow
because hsize is capped at 1,048,576 early in the function.
Signed-off-by: Srujith A Poojary <poojarysrujith@gmail.com>
---
[Outreachy PATCH] diff-delta: use safe integer arithmeticPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2276%2Fpoojarysrujith%2Fc-refactor-target-name-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2276/poojarysrujith/c-refactor-target-name-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/2276
Range-diff vs v4:
1: 0d5a729cde ! 1: 23fcfd3227 diff-delta: use safe integer arithmetic
@@ Metadata
## Commit message ##
diff-delta: use safe integer arithmetic
- Replace manual multiplication and addition in create_delta_index()
- with safe integer helpers (st_mult, st_add, st_add3) to prevent potential
- overflows when computing memory allocation sizes.
-
- For the unpacked index, we allocate hsize instances of hash pointers and
- entries instances of struct index_entry:
- - hsize is capped at 2^26.
- - entries is capped at 2^28.
- On 32-bit platforms with 4-byte padding for struct unpacked_index_entry,
- the calculation size (sizeof(*hash) * hsize + sizeof(*entry) * entries) can
- exceed 2^32, making safe integer arithmetic necessary.
-
- Similarly, for the packed index, we compute allocation size across
- struct index, packed_hash, and entries. Using safe helpers prevents
- wrap-around across both allocations.
-
- Note that passing (hsize + 1) as an argument to st_add3() is safe from
- overflow because hsize is capped at 2^26 early in the function.
+ In create_delta_index(), buffer sizes for allocating the delta index
+ are calculated using arithmetic operations on entries and hsize:
+
+ memsize = sizeof(*hash) * hsize + sizeof(*entry) * entries
+
+ To prevent potential size_t wrap-around when computing allocation sizes
+ on 32-bit systems, replace manual arithmetic with overflow-checked
+ helpers (st_mult, st_add, st_add3).
+
+ For the unpacked index allocation:
+ - sizeof(struct unpacked_index_entry) is 12 bytes on 32-bit systems,
+ which may be padded to 16 bytes depending on compiler alignment rules.
+ - sizeof(struct index_entry *) is 4 bytes.
+ - entries is capped by (0xffffffu / RABIN_WINDOW) where RABIN_WINDOW is 16,
+ limiting entries to 1,048,575.
+ - hsize is the smallest power of 2 greater than or equal to entries,
+ capped at 1,048,576.
+
+ While entries is bounded in current code, using st_mult() and st_add()
+ guarantees that any unexpected growth in struct sizes or bounds changes
+ will safely die() on size_t overflow instead of wrapping around.
+
+ For the packed index allocation:
+ - Passing (hsize + 1) as an argument to st_add3() is safe from overflow
+ because hsize is capped at 1,048,576 early in the function.
Signed-off-by: Srujith A Poojary <poojarysrujith@gmail.com>
diff-delta.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
Show changes to diff-delta.c +5 −5
diff --git a/diff-delta.c b/diff-delta.c
index 7cbedeb507..efebce3aca 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -162,8 +162,8 @@ struct delta_index * create_delta_index(const void *buf, size_t bufsize)
hmask = hsize - 1;
/* allocate lookup index */
- memsize = sizeof(*hash) * hsize +
- sizeof(*entry) * entries;
+ memsize = st_add(st_mult(sizeof(*hash), hsize),
+ st_mult(sizeof(*entry), entries));
mem = malloc(memsize);
if (!mem)
return NULL;
@@ -258,9 +258,9 @@ struct delta_index * create_delta_index(const void *buf, size_t bufsize)
* Now create the packed index in array form
* rather than linked lists.
*/
- memsize = sizeof(*index)
- + sizeof(*packed_hash) * (hsize+1)
- + sizeof(*packed_entry) * entries;
+ memsize = st_add3(sizeof(*index),
+ st_mult(sizeof(*packed_hash), hsize + 1),
+ st_mult(sizeof(*packed_entry), entries));
mem = malloc(memsize);
if (!mem) {
free(hash);
base-commit: 47fdf2208c5b94c8cc01c106b8f7c85efb91c926
