Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit the nesting depth and size of documents in Horizon #613

Open
danielmewes opened this issue Jun 23, 2016 · 6 comments
Open

Limit the nesting depth and size of documents in Horizon #613

danielmewes opened this issue Jun 23, 2016 · 6 comments

Comments

@danielmewes
Copy link
Member

RethinkDB should generally be able to handle documents of arbitrary nesting depth and/or size, but I wouldn't be shocked if you could somehow prepare a document that causes a stack overflow when being processed.

Additionally, very large documents could be used by an attacker for DoS attacks by causing high memory consumption on the server and/or slowing queries and other users down significantly.

I think we should implement a restriction on both aspects of any document that Horizon stores. The limits should probably be configurable for a given Horizon server instance.

I suggest default limits of:

  • 16 for the maximum nesting depth
  • 1 MB for the document size

Maybe there's a better way of achieving the same goal though?

I think we should consider adding this for 2.0. @Tryneus @deontologician ?

@danielmewes danielmewes added this to the Triaging... milestone Jun 23, 2016
@deontologician
Copy link
Contributor

This seems like it might want to be a security/permissions feature, but beyond that I don't have a strong opinion.

@mlucy
Copy link
Member

mlucy commented Jun 25, 2016

If the goal is to avoid stack overflows, note that we should also be careful about array nesting.

@sjmcdowall
Copy link

My question -- what's the contract between the caller and the server if a specific query exceeds either of those limits? Also -- is the nesting depth that critical or is it really the size that matters? :) If I nest 256 -- but my overall size is only 250K, is it that big of a deal? I'm a RethinkDB newbie so not sure if there are implications of nesting depth I am not aware of.

@deontologician
Copy link
Contributor

Since this isn't a breaking change, I think we can put it in post-2.0 for the sake of getting that release out faster

@deontologician deontologician modified the milestones: First post-2.0 release, Triaging... Jul 7, 2016
@deontologician
Copy link
Contributor

(This doesn't change the api, but of course it's breaking if people were adding huge documents)

@danielmewes
Copy link
Member Author

danielmewes commented Jul 8, 2016

@sjmcdowall The nesting depth is more important than the size of the individual values. Very large values can still be an issue, because they cause high memory consumption.

I have a slight preference for enforcing this even before we apply the normal security rules, since to check the security rules we'll already be processing the data. Who knows if some call in a security rule or in a validation function might itself have a limit on nesting depth, or use a lot of RAM if the documents are really large.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants