This repository has been archived by the owner. It is now read-only.

patch v8 (yeeees, I know, policy is not to do it) to prevent Hash-DoS #2431

Closed
thejh opened this Issue Dec 29, 2011 · 4 comments

Comments

Projects
None yet
2 participants
@thejh

thejh commented Dec 29, 2011

Hello,
I want to propose patching v8 to fix an important DoS issue. Yes, I know, it's nodejs policy to not alter v8 code, but this might be one case in which, in my opinion, an exception should be made. This issue allows an attacker to block your eventloop for quite a long time if you do require('querystring').parse(str) on a few megabytes of data or so (e.g. coming from an HTTP POST request).

Talk video: http://www.youtube.com/watch?v=R2Cq3CLI6H8

What needs to be done: Randomize the hash function (e.g. change the start value on startup to something random or so).

@thejh

This comment has been minimized.

Show comment
Hide comment
@thejh

thejh Dec 29, 2011

Just based on the public data, I was able to reprocude this somewhat - there are VERY simple and obvious collisions in the hash function (every coder should be able to recognize why if you show him the code). So, I made a bunch of object keys (10000) for the collision and did the same for something without the collision. 400-500 ms for creating the non-collision object, 7000-11000 ms for the object with collision.

thejh commented Dec 29, 2011

Just based on the public data, I was able to reprocude this somewhat - there are VERY simple and obvious collisions in the hash function (every coder should be able to recognize why if you show him the code). So, I made a bunch of object keys (10000) for the collision and did the same for something without the collision. 400-500 ms for creating the non-collision object, 7000-11000 ms for the object with collision.

@thejh

This comment has been minimized.

Show comment
Hide comment
@thejh

thejh Dec 29, 2011

The demo code is super-simple, I'll be happy to give it to every core dev.

thejh commented Dec 29, 2011

The demo code is super-simple, I'll be happy to give it to every core dev.

@thejh

This comment has been minimized.

Show comment
Hide comment
@thejh

thejh Dec 29, 2011

Hmm, djb says randomizing isn't enough.

https://twitter.com/#!/hashbreaker/status/152337809527668736

https://twitter.com/#!/hashbreaker/status/152070032908750851

Sounds reasonable... an attacker could still probe the server for collisions, calculate the randomization from it and attack that hash function.

thejh commented Dec 29, 2011

Hmm, djb says randomizing isn't enough.

https://twitter.com/#!/hashbreaker/status/152337809527668736

https://twitter.com/#!/hashbreaker/status/152070032908750851

Sounds reasonable... an attacker could still probe the server for collisions, calculate the randomization from it and attack that hash function.

@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Feb 18, 2012

Member

Hash randomization was added in 4a899c9.

Member

bnoordhuis commented Feb 18, 2012

Hash randomization was added in 4a899c9.

@bnoordhuis bnoordhuis closed this Feb 18, 2012

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.