Skip to content

fix: strengthen Map/Set key hashing against collision DoS#1556

Open
mvanhorn wants to merge 2 commits into
quickjs-ng:masterfrom
mvanhorn:fix/205-map-set-hash-collisions
Open

fix: strengthen Map/Set key hashing against collision DoS#1556
mvanhorn wants to merge 2 commits into
quickjs-ng:masterfrom
mvanhorn:fix/205-map-set-hash-collisions

Conversation

@mvanhorn

@mvanhorn mvanhorn commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Strengthens the hashing of Map/Set keys against collision-based denial of service. String keys with embedded NUL bytes and float64 keys with adversarial bit patterns previously funneled into a single bucket, making insertion quadratic. Keys now hash with an avalanche mix so bucket distribution stays even under attacker-chosen inputs.

Why this matters

Issue #205 reports that Map/Set insertion can be driven quadratic with crafted keys. Two families were exploitable: strings sharing a prefix up to a NUL byte, and float64 values whose 32-bit halves are equal and share low bits (easy to generate via a DataView). Both collapsed to the same low hash bits, so once the table grew to 4096 buckets they all landed in one bucket and each insert walked the whole chain.

The string path is hardened, and the float64 key hash now runs the 64-bit pattern through a multiply/xorshift avalanche folded to 32 bits before the bucket mask, so equal-word / shared-low-bit families no longer collide. NaN normalization and +0/-0 unification are preserved.

Testing

make, make test (Result: 0/64 errors), and build/run-test262 -c tests.conf -f tests/test_builtin.js all pass. tests/test_builtin.js gains regression coverage for NUL-prefixed string keys, the equal-word float collision family at 4096 entries (both Map and Set), NaN, and +0/-0.

Fixes #205

Comment thread quickjs.c
@@ -52665,6 +52665,16 @@ static JSValueConst map_normalize_key_const(JSContext *ctx, JSValueConst key)
}

/* XXX: better hash ? */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

time to drop the comment now?

Comment thread quickjs.c
}

/* XXX: better hash ? */
static uint32_t map_hash_u64(uint64_t h)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this some well-known formula? maybe add a comment with a reference pl

@saghul saghul requested a review from bnoordhuis July 2, 2026 11:21
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.

Set and Map are vulnerable to hash collisions

2 participants