google / libprotobuf-mutator

Library for structured fuzzing with protobuffers
Apache License 2.0
594 stars 120 forks source link

Recursive depth on google.protobuf.Structs #143

Open asraa opened 5 years ago

asraa commented 5 years ago

Hi,

It seems like one of our fuzzers is failing because of a stack-overflow in protobuf's TextFormat::Parser. The input that libprotobuf-mutator created was a Struct that has a large recursive depth, but is not enough to trigger the max recursion limit in the parser (which is by default 2^15 - 1). https://github.com/protocolbuffers/protobuf/blob/6fc04c3f0e07857dff1f55b884f957b4c65aea8e/src/google/protobuf/text_format.cc#L1383.

Is there some environment variable we could set to limit the recursive depth that LPM creates, or another way to handle such an input?

Thanks!

vitalybuka commented 4 years ago

I assumed I solved the issue few month ago https://github.com/google/libprotobuf-mutator/blob/dd89da92b59b1714bab6e2a135093948a1cf1c6a/src/text_format.cc#L31

Is possible that just 100 breaks your binary?

dgg5503 commented 6 months ago

Hi @vitalybuka,

Apologies for bumping a 5-year-old thread, however, I believe I've encountered an issue like this one. When using LPM integrated with libFuzzer, it is possible for LPM to generate a deeply nested message beyond the limits of the text or binary parsers. This is problematic in the following situation:

In this situation, one may increase the recursion limit via SetRecursionLimit, however, even deeper messages are still capable of being generated and saved to disk, especially via crossover. Additionally, the fuzzer and possibly the function under test may end up spending more time deserializing, processing, & mutating deeply nested messages which could be better spent mutating fields at shallower levels.

With that said, is there any way to enforce a depth limit at the mutation level that I may have overlooked? In other words, during LPM mutation, if it detects that an add/clone/copy will put the mutated message over some user-provided maximum depth, can that be prevented in favor of shallower field mutation? I believe this would:

If there isn't already a way to enforce this, I'm curious what you'd think would be required to implement such a feature. I would be interested in providing a PR to add such functionality.

I've attached a simple example which demonstrates the scenario above. issue-143-supplement.tar.gz

Thank you.