Avoid hitting some known minor tinyformat issues when fuzzing strprintf(...). These can be removed when the issues have been resolved upstreams :)
Note to reviewers: The %c and %* issues are also present for %<some junk>c and %<some junk>*. That is why simply matching on "%c" or "%*" is not enough. Note that the intentionally trivial skipping logic overshoots somewhat (c[…]% is filtered in addition to %[…]c).
114 | @@ -115,28 +115,50 @@ void test_one_input(const std::vector<uint8_t>& buffer) 115 | case 5: 116 | (void)strprintf(format_string, fuzzed_data_provider.ConsumeBool()); 117 | break; 118 | - case 6: 119 | + } 120 | + } catch (const tinyformat::format_error&) { 121 | + } 122 | + 123 | + if (format_string.find("%") != std::string::npos && format_string.find("c") != std::string::npos) {
This not just matches %<some junk>c, but also c<some junk>%, since both searches start from the begin of the format string. To correct that you'd need to save the found position of the % (=return value of the first .find()) and pass it to the search for the c (=second parameter of the second .find()). Given that if I understood your intention correctly ;-)
Also, a tiny nit: could use the overloaded version of .find() which takes a char as first parameter instead of passing strings with length 1, see http://www.cplusplus.com/reference/string/string/find/ (probably more efficient).
See comment below :)
125 | + // * strprintf("%c", 1.31783e+38); 126 | + // tinyformat.h:244:36: runtime error: 1.31783e+38 is outside the range of representable values of type 'char' 127 | + return; 128 | + } 129 | + 130 | + if (format_string.find("%") != std::string::npos && format_string.find("*") != std::string::npos) {
See my review comment above (matching logic and as micro-nit using .find()-version with char as first parameter).
I usually avoid to do code-reviews before I can give at least "Concept ACK". In this case I unfortunately don't have enough clue about the details of fuzzing, but saw that the matching logic doesn't correspond to the desired effect in the PR description. Hope that's still okay and helpful.
@theStack Thanks for the review! I'll address the nit about .find(char). The skipping logic is intentional - I wanted to keep it as simple as possible and the overshoot (that c[…]% is matched in addition to %[…]c) doesn't matter in practice. I'm happy to change that if proven wrong or if anyone cares strongly :) Regardless I'll update the PR description to make the intention more clear. Again, thanks for reviewing!