-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace the hash function used in YJIT with a faster implementation #561
Comments
For reference, the 2 CRuby CVE about HashDoS:
And the move to SipHash-1-3: https://bugs.ruby-lang.org/issues/13017 The second CVE means just MurmurHash + a random seed wasn't enough, at least for hashing keys of user-facing Hash.
And I think that's why we still have quite expensive hash functions in CRuby these days and not just a random seed. FWIW Java or at least HotSpot chose to fix differently: java.lang.String#hashCode is still fully deterministic to this day but the hashtables implementations like java.util.HashMap switch internally to a tree-like comparison Map when they detect too many hash collisions. |
Using SipHash seems reasonable.
My research was insufficient. Thank you @eregon for your opinion. I will close this issue. |
Issue description:
Replace the hash function used in YJIT from the default SipHash in the standard library to a faster alternative.
Motivation:
YJIT currently uses HashMap and HashSet from the standard library, which utilize SipHash by default. While SipHash is collision-resistant, it is relatively slow. Replacing it with a faster hash function, even at the cost of reduced collision resistance, may improve performance.
Task:
Concerns:
Switching to a faster, non-collision-resistant hash function may increase the risk of hash flooding attacks. To mitigate this, careful implementation and random seeding will be required.
For example, Node.js faced a hash flooding vulnerability in the past: https://v8.dev/blog/hash-flooding.
Conclusion:
I welcome feedback, especially regarding potential security concerns or implementation suggestions. Thank you.
The text was updated successfully, but these errors were encountered: