Project

General

Profile

Bug #13376

Symbol#hash is deterministic on 2.3

Added by chrisseaton (Chris Seaton) 7 months ago. Updated 6 months ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 2.3.3p222 (2016-11-21 revision 56859) [x86_64-darwin16]
[ruby-core:80430]

Description

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

Associated revisions

Revision 58200
Added by normal 7 months ago

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 [Bug #13376]

Revision 58203
Added by nagachika (Tomoyuki Chikanaga) 7 months ago

merge revision(s) 58200: [Backport #13376]

* hash.c (any_hash): fix Symbol#hash to be nondeterministic.
  The patch was provided by Eric Wong.  [Bug #13376]

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
   [Bug #13376]

Revision 58213
Added by nagachika (Tomoyuki Chikanaga) 7 months ago

  • hash.c (any_hash): fix CI failure on L32LLP64 architecture. The patch was provided by usa. [Bug #13376]

History

#1 [ruby-core:80431] Updated by normalperson (Eric Wong) 7 months ago

chris@chrisseaton.com 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.

#2 [ruby-core:80433] Updated by normalperson (Eric Wong) 7 months ago

Here is a 2.3-only patch.

#3 Updated by Anonymous 7 months ago

  • 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 [Bug #13376]

#4 [ruby-core:80434] Updated by normalperson (Eric Wong) 7 months ago

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

#5 [ruby-core:80435] Updated by Eregon (Benoit Daloze) 7 months ago

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.

#6 [ruby-core:80439] Updated by nagachika (Tomoyuki Chikanaga) 7 months ago

  • 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.

#7 [ruby-core:80480] Updated by darix (Marcus Rückert) 7 months ago

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

#8 [ruby-core:80482] Updated by nobu (Nobuyoshi Nakada) 7 months ago

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

#9 [ruby-core:80483] Updated by Eregon (Benoit Daloze) 7 months ago

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.

#10 [ruby-core:80484] Updated by usa (Usaku NAKAMURA) 7 months ago

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

#11 [ruby-core:80485] Updated by nagachika (Tomoyuki Chikanaga) 7 months ago

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.

#12 Updated by usa (Usaku NAKAMURA) 6 months ago

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

Also available in: Atom PDF