hash: vary bucket placement across process runs

[MegaManSec]

hash_str() always starts from the same value, so a given key set lands in the same buckets on every run. Mix in a random seed generated at global init.

This does not reduce the collision probability or change the worst case; it only stops a clustering from repeating between runs.

unit1603 covers the seeded calculation.

[bagder]

Good idea!

[Copilot]

🟡 Changes recommended

Random-generation failure permanently disables the mitigation, and the seeded path lacks test coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds process-lifetime randomization to byte-key hash bucket selection to reduce repeatable clustering.

Changes:

• Initializes a random hash seed during global setup.
• Mixes the seed into string hashing.
• Adds required declarations.

File summaries

File	Description
lib/rand.h	Forward-declares Curl_easy.
lib/hash.h	Declares hash global initialization.
lib/hash.c	Generates and applies the hash seed.
lib/easy.c	Initializes the seed during global setup.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jay]

Couldn't we have the same issue whether there's a random seed or not. Like it's already random in a sense.

[MegaManSec]

@jay it doesn't reduce the normal collision probability or change the O(n) worst case, but it makes bucket placement vary between runs, so any that happens clustering isn't guaranteed to repeat. updated pr description to be a bit moreclear

[jay]

What I'm saying is it already isn't necessarily repeating because the input will vary but say you have the same command or input then yes with this pr the hash would vary the same strings between runs. I wonder why we should do that though if the input is already "random" even if it's the same over several runs.

[testclutch]

Analysis of PR #22916 at 2f40fa96:

Test 1603 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 46 different CI jobs (the link just goes to one of them).

Generated by Testclutch

[bagder]

The IntelC CI warning looks real.

Also, the mbedtls failures I believe is because the test case does not call curl_global_init() and for mbedTLS that means it didn't seed the random engine so things fail later.

[dfandrich]

I wonder why we should do that though if the input is already "random" even if it's the same over several runs.

I would think the impetus for this is to make it impossible for an attacker to precompute a worst-case set of strings that would then cause a DoS when fed to curl.