Mimic CRuby's numeric hashing#6305
Merged
headius merged 1 commit intojruby:jruby-9.2from Jun 30, 2020
Merged
Conversation
headius
added a commit
to headius/jruby
that referenced
this pull request
Jun 29, 2020
This corrects the error raised for the case where the unpack spec requests a size that's outside the string, which relates to CVE-2018-8778. The other two cases that raised the ArgumentError for "outside of string" appear to be correct, since making them raise RangeError causes spec failures. This addresses a security spec failure as shown in jruby#6305.
headius
force-pushed
the
numeric_rand_hash
branch
from
f224f3a to
067f752
Compare
CRuby modified the hashing of core numeric types to incorporate a random seed, as part of the flurry of "hashDOS" fixes in the early 2010s. This commit mimics those changes. See jruby#6304 for the failing security spec.
headius
force-pushed
the
numeric_rand_hash
branch
from
067f752 to
42ff965
Compare
Member
Author
|
I will modify this to put the random seed logic behind the same "consistent hash" option, so it can be disabled. |
Member
Author
|
Aha, the random seed property |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes changes to RubyFixnum, RubyBignum, and RubyFloat hashing to incorporate the per-runtime random seed, in an effort to mimic CRuby's fixes for CVE-2011-4815.
This CVE was part of a flurry of "hashDOS" fixes that happened in the early 2010s to address poor hash use by application frameworks like Rails. Because untrusted data from web requests was blindly used for internal hashing, these frameworks could be DOS'ed by predicting the hashes of strings and other data and tricking Ruby into putting all elements in the same linear-scanned hash box.
The change here roughly mimics the CRuby logic for combining a random seed with the 64-bit long representation of these core numeric types, which in turn propagates that randomness to the composite types like Complex and Rational.
We may want to make this randomization optional, since we have had requests like #590 to allow users to opt into repeatable hashing.
This PR can safely merge to master.
See #6304 for the security specs failing without this change. This PR addresses all the failing numeric hash security specs.