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

Fix stack overflow when parsing deeply nested configs #6325

Merged

Conversation

Smjert
Copy link
Member

@Smjert Smjert commented Mar 23, 2020

Parsing a configuration file as a JSON document
which contains deeply nested elements can lead to a stack overflow
when using the recursive parser of RapidJSON.
Since the configuration isn't changed or parsed frequently,
use the slower iterative parser instead.

Copying the configuration JSON document
that contains deeply nested elements, using the CopyFrom API,
can lead to a stack overflow, due to the recursive nature
of the RapidJSON GenericValue construction.
Detect the depth/nesting level of a config document
and limit it to 32 levels.

Using an iterative parser, while it avoids stack overflows,
can cause memory exhaustion if the config size is too big.
Limit the maximum config size, stripped from its comments, to 1MiB.

Addresses https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20779

Parsing a configuration file as a JSON document
which contains deeply nested elements can lead to a stack overflow
when using the recursive parser of RapidJSON.
Since the configuration isn't changed or parsed frequently,
use the slower iterative parser instead.

Copying the configuration JSON document
that contains deeply nested elements, using the CopyFrom API,
can lead to a stack overflow, due to the recursive nature
of the RapidJSON GenericValue construction.
Detect the depth/nesting level of a config document
and limit it to 32 levels.

Using an iterative parser, while it avoids stack overflows,
can cause memory exhaustion if the config size is too big.
Limit the maximum config size, stripped from its comments, to 1MiB.

Addresses https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20779
@lgtm-com
Copy link

lgtm-com bot commented Mar 24, 2020

LGTM pull request analysis was skipped for 8012f86 by Smjert. Analysis of future commits will happen as normal.

@theopolis theopolis merged commit e70de5b into osquery:master Mar 27, 2020
@Smjert Smjert deleted the stefano/fix/stack-overflow-decorators branch April 2, 2020 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants