Fuzzing Harness Leads to False Positives on Bad Format Strings

[clesmian]

The fuzzing harness used for OSS-Fuzz does not do any sanity check on the input to determine whether it contains any format characters. As it does not supply any arguments to redisFormatCommand this leads to crashes for numerous format strings like, for example, %s%s that require dereferencing pointers supplied as arguments.

Crashes like this are however not actual issues, because they do not result from a bug in the tested code, but rather a bug in the fuzzer, namely failing to supply the correct number of arguments to the function.

Possible Ways Forward

To improve the situation I see two ways forward:

1. 'Defuse' the format strings by replacing % with a different character OR
2. Count the amount of format specifiers and supply a sufficient amount of arguments (potentially taken from the fuzzing input)

Doing so would improve the quality of the harness, because it no longer crashes for obvious non-issues.

[michael-grunder]

I noticed this some time ago and had planned on fixing it but other things got in the way.

My instinct is to just diffuse the format strings. We could attempt to count and provide sufficient arguments, but the logic would need to both count the specifiers and be completely aware of what a valid argument would look like. For example %s' would require a null terminated string and %b` requires the input length.

[clesmian]

I'm sorry, it completely slipped my mind to answer. To get better coverage, one could do a mix of both. I.e. iterate over all occurrences of possible format specifiers, add an argument for those specifiers that are "trivial" to support (e.g. %s) and disarm the rest.

What do you think?

[michael-grunder]

@JJLeo I linked your fuzzing issues to this false positive thread.

The problem is that generating randomized format strings is kind of not inherently "fuzzable" since it fuzzes a variadic function. As you can see we discussed a few strategies to improve the situation in here. I'll play around with it this week and see if I can come up with a middle ground so we can continue to derive benefit from the fuzzer.

[michael-grunder]

This was actually resolved in #1295