Bug #13376: Symbol#hash is deterministic on 2.3

• Status: Closed
• Priority: Normal
• Assignee: -
• Target version: -
• ruby -v: ruby 2.3.3p222 (2016-11-21 revision 56859) [x86_64-darwin16]
• Backport: 2.2: DONTNEED, 2.3: DONE, 2.4: UNKNOWN

[chrisseaton (Chris Seaton)]

I believe the Symbol#hash should probably be non-deterministic, due to CVE-2011-4815. That seems to be the behaviour on 2.2 and 2.4, but not on 2.3. Was this a conscious decision at the time? Or is it a bug?

$ 2.2.6/bin/ruby -e 'puts :foo.hash'
-505215953858886063

$ 2.2.6/bin/ruby -e 'puts :foo.hash'
3929535091178311289

$ 2.3.3/bin/ruby -e 'puts :foo.hash'
2810

$ 2.3.3/bin/ruby -e 'puts :foo.hash'
2810

$ 2.4.0/bin/ruby -e 'puts :foo.hash'
-1200094397129038718

$ 2.4.0/bin/ruby -e 'puts :foo.hash'
-916960310565036298

[normalperson (Eric Wong)]

[e-mail] wrote:

Bug #13376: Symbol#hash is deterministic on 2.3
https://bugs.ruby-lang.org/issues/13376

I think I broke this in:

commit 14470aa6dbf4d99bc8e0484e1334c2c6d5e68fc3 / r51582
("hash.c: improve integer/fixnum hashing")

and nobu fixed this in:

commit a49f6016ea48a40865c91137b33ff9115e4071fd / r56992
("switching hash removal")

Which was too big to backport... I will come up with a 2.3-only fix.

[normalperson (Eric Wong)]

• Backport changed from 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN to 2.2: UNKNOWN, 2.3: REQUIRED, 2.4: UNKNOWN
• File 0001-hash.c-any_hash-make-static-symbol-hash-non-determin.patch 0001-hash.c-any_hash-make-static-symbol-hash-non-determin.patch added

Here is a 2.3-only patch.

• Status changed from Open to Closed

Applied in changeset trunk|r58200.

---

test/ruby/test_symbol.rb: new test for nondeterminism

We need to ensure hashes for static symbols remain
non-deterministic to avoid DoS attacks. This is currently the
case since 2.4+, but was not for the 2.3 series.

• test/ruby/test_symbol.rb (test_hash_nondeterministic): new test [ruby-core:80430] [Bug #13376]

[normalperson (Eric Wong)]

I also committed r58200 to trunk to prevent us from hitting
this, again. We should ensure other classes don't suffer this
fate, too, will check other cases later (or if other people send
patches).

/me goes back to hibernation

[Eregon (Benoit Daloze)]

For information, https://github.com/ruby/spec/pull/393 is adding such tests in ruby/spec
for Object, Integer, Float, String, Symbol, Array and Hash.
That's how the bug was discovered.

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.2: UNKNOWN, 2.3: REQUIRED, 2.4: UNKNOWN to 2.2: UNKNOWN, 2.3: DONE, 2.4: UNKNOWN

Thank you Chris for your report. And thank you Eric creating a patch for ruby_2_3!

I backported r58200 with Eric's patch into ruby_2_3 branch at r58203.

[darix (Marcus Rückert)]

should this receive a new CVE?
should this released be soon as 2.3.4?

[nobu (Nobuyoshi Nakada)]

Accepting huge requests which could exhaust memory with too may symbols at once would be rarely possible in 2.3.

[Eregon (Benoit Daloze)]

nobu (Nobuyoshi Nakada) wrote:

Accepting huge requests which could exhaust memory with too may symbols at once would be rarely possible in 2.3.

CVE-2011-4815 is about hash collisions, which indeed seems possible if a user can control Symbol keys inserted into a Hash in 2.3.

[usa (Usaku NAKAMURA)]

I've fixed it.
nagachika-san, please apply this patch:

Index: hash.c
===================================================================
--- hash.c  (revision 58210)
+++ hash.c  (working copy)
@@ -168,7 +168,7 @@ any_hash(VALUE a, st_index_t (*other_func)(VALUE))
     }
   out:
     hnum <<= 1;
-    return (st_index_t)RSHIFT(hnum, 1);
+    return (long)RSHIFT(hnum, 1);
 }

 static st_index_t

[nagachika (Tomoyuki Chikanaga)]

Thank you usa-san. I'll merge your patch soon.
I'd like to make v2_3_4 tag after confirming the result of CI on vc12-x64.

[usa (Usaku NAKAMURA)]

• Backport changed from 2.2: UNKNOWN, 2.3: DONE, 2.4: UNKNOWN to 2.2: DONTNEED, 2.3: DONE, 2.4: UNKNOWN