[Security] KeySpaceConstrainedParams useless in its current implementation? #356

Closed
tizoc opened this Issue Mar 5, 2012 · 4 comments

Projects

None yet

2 participants

@tizoc

KeySpaceConstrainedParams is supposed to prevent the application from clients that send too many keys+values by limiting the amount of space they can take up. I don't think that this is the correct approach, and also, in its current version it isn't very useful because anyone who wants to do damage can circumvent the protection mechanism with ease being that nesting is supported.

The code in question is:

https://github.com/rack/rack/blob/master/lib/rack/utils.rb#L56
https://github.com/rack/rack/blob/master/lib/rack/utils.rb#L436

The current limit applies to every KeySpaceConstrainedParams instance in isolation, but the childrens are not being considered, which means that for the possible resulting structures, the limit is actually much higher (actually, it is infinite, there is no limit when nesting is involved)

@tizoc

My opinion is that having this isn't really useful, and should be handled by any server that sits in-front of our app (nginx, apache, whatever).

@rkh
Official Rack repositories member
rkh commented Mar 5, 2012

This is supposed to protect again a vulnerability in older Ruby versions. Nesting is not an issue either. This is not supposed to save memory but to avoid hash collisions (where you can keep ruby calculating and comparing bits for hours with just a 5 MB payload).

@tizoc

Fair enough, thanks for the explaination. Lets say I wanted to implement params using a Flask-like MultiOrderectDict implementation (as a possible to get what I'm asking for in #355), would you say that I should still have to take care of this?

Do you remember which versions of Ruby are vulnerable?

@tizoc tizoc closed this Mar 5, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment