-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Description
malloc accepts a size_t argument, our own wrappers like mem_alloc and others in memory.[ch] do as well. On 32-bit platforms, size_t is typically 32-bit as well. However, we have calls that pass 64-bit values in there, which in some cases may actually exceed what fits in 32 bits. When this happens, we attempt allocating an incorrect (too small) amount of memory, which might succeed, followed by a write of potentially the larger amount of data (although this is also subject to potential truncation to size_t, in other calls).
For example, in zip2john.c we have:
p->hash_data = mem_alloc(p->cmp_len + 1);
if (fread(p->hash_data, 1, p->cmp_len, fp) != p->cmp_len) {where:
uint64_t cmp_len, decomp_len;since fread also accepts a size_t, for most values we don't actually have an out of bounds write into the memory here, and the under-read is detected through the != p->cmp_len check. However, for cmp_len of 0xffffffff, we do have the problem. (BTW, why do we even allocate the extra 1 byte here.)
We could want to introduce some way to reliably fail allocations of larger than 32-bit sizes when size_t is 32-bit.
Then, in fgetll() we have:
new_cp = realloc(cp, len + increase);
I guess this can overflow a 32-bit size_t, too, and we need to pre-check for that.
There are probably more problematic cases like this.