Skip to content

map: fix uint32 overflow in sc_map_alloc when init cap is huge - #139

Merged
tezc merged 1 commit into
tezc:masterfrom
94xhn:fix/sc-map-alloc-overflow
Jul 17, 2026
Merged

map: fix uint32 overflow in sc_map_alloc when init cap is huge#139
tezc merged 1 commit into
tezc:masterfrom
94xhn:fix/sc-map-alloc-overflow

Conversation

@94xhn

@94xhn 94xhn commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What

sc_map_alloc_##name() (used by sc_map_init_*() with factor == 1)
rounds the requested capacity up to the next power of two. When cap
is large enough (roughly > 2^31), that round-up wraps uint32_t
back to 0 instead of failing.

The existing overflow guard

if (*cap > SC_MAP_MAX / factor) {
    return NULL;
}

can't catch this on the sc_map_init_*() path because factor is
1 there, so the check degenerates to cap > UINT32_MAX, which can
never be true for a uint32_t value.

Impact

sc_map_init_*() returns true with m->cap silently set to 0
(while still allocating a real, tiny 1-item buffer). Any later
sc_map_get_*() / sc_map_del_*() call computes
mod = m->cap - 1, which underflows to UINT32_MAX, and then
indexes m->mem[pos] with pos derived from a full 32-bit hash -
an out-of-bounds access far past the actual allocation.

Reproduced (unmodified map/sc_map.c, gcc, x86_64):

struct sc_map_str map;
sc_map_init_str(&map, 2147483649U, 0); /* returns true, map.cap == 0 */
sc_map_get_str(&map, "somekey");       /* segfaults */

sc_map_put_*() is not affected the same way in practice, because it
calls sc_map_remap_##name() first, and remap (which is 0 in
this corrupted state) immediately forces a real grow-to-8 before any
indexing happens - get/del have no such self-healing step.

Fix

Check for the wrap directly on the rounded-up value, right after
computing it, instead of relying on the pre-multiply guard. This
covers the factor == 1 (init) path that the existing check misses,
and is a no-op for the factor == 2 (remap/grow) path, where the
existing guard already works correctly.

Testing

  • Added a small standalone repro harness confirming the segfault on
    the unmodified code, and confirming sc_map_init_str() now returns
    false cleanly for the same input instead of corrupting state.
  • Ran the existing map/map_test.c against the patched file with
    -Wall -Wextra -pedantic -std=c99 - builds clean, passes.
  • Wrote a differential fuzz harness comparing sc_map_32 against a
    naive reference map across ~8M randomized put/get/del/foreach
    operations (small keyspace to force heavy collisions/wraparound,
    plus a larger keyspace that forces multiple organic remap grows,
    including key 0) - zero mismatches before and after the patch, so
    the fix doesn't change behavior for any valid capacity.

No functional change for any cap value that doesn't already trigger
the overflow.

sc_map_alloc_##name() rounds the requested capacity up to the next
power of two. When cap is large enough (roughly > 2^31), the round-up
wraps uint32_t back to 0 instead of failing.

The existing overflow guard 'if (*cap > SC_MAP_MAX / factor)' cannot
catch this on the sc_map_init_*() path because factor is 1 there, so
the check degenerates to 'cap > UINT32_MAX', which is never true for
a uint32_t value.

The result: sc_map_init_*() returns true with m->cap silently set to
0 (while still allocating a 1-item buffer). Any later
sc_map_get_*()/sc_map_del_*() call then computes
'mod = m->cap - 1', which underflows to UINT32_MAX, and indexes
m->mem[pos] with pos derived from a full 32-bit hash - an
out-of-bounds access far past the tiny actual allocation.

Reproduced with:

    struct sc_map_str map;
    sc_map_init_str(&map, 2147483649U, 0); // returns true, map.cap == 0
    sc_map_get_str(&map, "somekey");        // segfaults

Fix: check for the wrap directly after computing the rounded-up
value, regardless of which factor produced it.
@tezc
tezc merged commit d0f6a80 into tezc:master Jul 17, 2026
19 of 20 checks passed
@tezc

tezc commented Jul 17, 2026

Copy link
Copy Markdown
Owner

@94xhn thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants