Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upInitial fuzzing with libFuzzer - memory leak, UB and buffer write overflow #345
Labels
Comments
This comment has been minimized.
This comment has been minimized.
|
Let us investigate. |
This comment has been minimized.
This comment has been minimized.
|
Your test revealed a potential memory leak when parsing an input that fails stage 1, fixed in the following commit : (The fix is trivial and so is the issue, once it is spotted.) |
This comment has been minimized.
This comment has been minimized.
|
I'll open a distinct issue for the buf overflow. |
This comment has been minimized.
This comment has been minimized.
|
Buf overflow fixed. See related PR. |
This comment has been minimized.
This comment has been minimized.
|
UB fixed via da1c35d |
This comment has been minimized.
This comment has been minimized.
|
It might be worth it to submit as a PR your fuzzer to the project. You caught three bugs that had escaped us thus far. Well done. |
This comment has been minimized.
This comment has been minimized.
|
Thanks! I added the fuzzer cmake boilerplate in a way such that it is possible to have it being built by oss-fuzz. That change may not be what you want, but I am not sure the existing cmake is well behaved. The reason is that (if I got this right) you are supposed to be able to pass compiler flags and the build should respect them. As it is now on master, CXXFLAGS are replaced. oss-fuzz passes flags via CXXFLAGS and the fuzzer via a linker flag. I solved the latter for fmt like this: https://github.com/fmtlib/fmt/blob/b7a157401e54895b491d0189045f27ca5cf100f6/test/fuzzing/CMakeLists.txt#L31 which works fine. |
Merged
This comment has been minimized.
This comment has been minimized.
|
Improvements to our CMake setup are invited. |
piotte13
added a commit
that referenced
this issue
Nov 7, 2019
* rough prototype working. Needs more test and fine tuning. * prototype working on large files. * prototype working on large files. * Adding benchmarks * jsonstream API adjustment * type * minor fixes and cleaning. * minor fixes and cleaning. * removing warnings * removing some copies * runtime dispatch error fix * makefile linking src/jsonstream.cpp * fixing arm stage 1 headers * fixing stage 2 headers * fixing stage 1 arm header * making jsonstream portable * cleaning imports * including <algorithms> for windows compiler * cleaning benchmark imports * adding jsonstream to amalgamation * merged main into branch * bug fix where JsonStream would bug on rare cases. * Addind a JsonStream Demo to Amalgamation * Fix for #345 * Follow up test and fix for #345 (#347) * Final (?) fix for #345 * Verbose basictest * Being more forgiving of powers of ten. * Let us zero the tail end. * add basic fuzzers (#348) * add basic fuzzing using libFuzzer * let cmake respect cflags, otherwise the fuzzer flags go unnoticed also, integrates badly with oss-fuzz * add new fuzzer for minification, simplify the old one * add fuzzer for the dump example * clang format * adding Paul Dreik * rough prototype working. Needs more test and fine tuning. * prototype working on large files. * prototype working on large files. * Adding benchmarks * jsonstream API adjustment * type * minor fixes and cleaning. * Fixing issue 351 (#352) * Fixing issues 351 and 353 * minor fixes and cleaning. * removing warnings * removing some copies * Fix ARM compile errors on g++ 7.4 (#354) * Fix ARM compilation errors * Update singleheader * runtime dispatch error fix * makefile linking src/jsonstream.cpp * fixing arm stage 1 headers * fixing stage 2 headers * fixing stage 1 arm header * fix integer overflow in subnormal_power10 (#355) detected by oss-fuzz https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=18714 * Adding new test file, following #355 * making jsonstream portable * cleaning imports * including <algorithms> for windows compiler * cleaning benchmark imports * adding jsonstream to amalgamation * merged main into branch * bug fix where JsonStream would bug on rare cases. * Addind a JsonStream Demo to Amalgamation * merging main * rough prototype working. Needs more test and fine tuning. * prototype working on large files. * prototype working on large files. * Adding benchmarks * jsonstream API adjustment * minor fixes and cleaning. * minor fixes and cleaning. * removing warnings * removing some copies * runtime dispatch error fix * makefile linking src/jsonstream.cpp * fixing arm stage 1 headers * fixing stage 2 headers * fixing stage 1 arm header * making jsonstream portable * cleaning imports * including <algorithms> for windows compiler * cleaning benchmark imports * adding jsonstream to amalgamation * bug fix where JsonStream would bug on rare cases. * Addind a JsonStream Demo to Amalgamation * rough prototype working. Needs more test and fine tuning. * minor fixes and cleaning. * adding jsonstream to amalgamation * merged main into branch * Addind a JsonStream Demo to Amalgamation * merging main * merging main * make file fix
piotte13
added a commit
that referenced
this issue
Nov 8, 2019
* rough prototype working. Needs more test and fine tuning. * prototype working on large files. * prototype working on large files. * Adding benchmarks * jsonstream API adjustment * type * minor fixes and cleaning. * minor fixes and cleaning. * removing warnings * removing some copies * runtime dispatch error fix * makefile linking src/jsonstream.cpp * fixing arm stage 1 headers * fixing stage 2 headers * fixing stage 1 arm header * making jsonstream portable * cleaning imports * including <algorithms> for windows compiler * cleaning benchmark imports * adding jsonstream to amalgamation * merged main into branch * bug fix where JsonStream would bug on rare cases. * Addind a JsonStream Demo to Amalgamation * Fix for #345 * Follow up test and fix for #345 (#347) * Final (?) fix for #345 * Verbose basictest * Being more forgiving of powers of ten. * Let us zero the tail end. * add basic fuzzers (#348) * add basic fuzzing using libFuzzer * let cmake respect cflags, otherwise the fuzzer flags go unnoticed also, integrates badly with oss-fuzz * add new fuzzer for minification, simplify the old one * add fuzzer for the dump example * clang format * adding Paul Dreik * rough prototype working. Needs more test and fine tuning. * prototype working on large files. * prototype working on large files. * Adding benchmarks * jsonstream API adjustment * type * minor fixes and cleaning. * Fixing issue 351 (#352) * Fixing issues 351 and 353 * minor fixes and cleaning. * removing warnings * removing some copies * Fix ARM compile errors on g++ 7.4 (#354) * Fix ARM compilation errors * Update singleheader * runtime dispatch error fix * makefile linking src/jsonstream.cpp * fixing arm stage 1 headers * fixing stage 2 headers * fixing stage 1 arm header * fix integer overflow in subnormal_power10 (#355) detected by oss-fuzz https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=18714 * Adding new test file, following #355 * making jsonstream portable * cleaning imports * including <algorithms> for windows compiler * cleaning benchmark imports * adding jsonstream to amalgamation * merged main into branch * bug fix where JsonStream would bug on rare cases. * Addind a JsonStream Demo to Amalgamation * merging main * rough prototype working. Needs more test and fine tuning. * prototype working on large files. * prototype working on large files. * Adding benchmarks * jsonstream API adjustment * minor fixes and cleaning. * minor fixes and cleaning. * removing warnings * removing some copies * runtime dispatch error fix * makefile linking src/jsonstream.cpp * fixing arm stage 1 headers * fixing stage 2 headers * fixing stage 1 arm header * making jsonstream portable * cleaning imports * including <algorithms> for windows compiler * cleaning benchmark imports * adding jsonstream to amalgamation * bug fix where JsonStream would bug on rare cases. * Addind a JsonStream Demo to Amalgamation * rough prototype working. Needs more test and fine tuning. * minor fixes and cleaning. * adding jsonstream to amalgamation * merged main into branch * Addind a JsonStream Demo to Amalgamation * merging main * merging main * make file fix
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I added a very basic fuzzing attempt, more or less from the README on the front page. It found a memory leak instantly. This is why I proceeded with running the normal tool in valgrind (see #340).
The fuzzer is available at https://github.com/pauldreik/simdjson/tree/paul/initialfuzz/fuzz
Here is what I did:
and this is the first result (memory leak)
It also finds UB, if one supress the memory leak detection and let it proceed anyway
And here is the buffer overflow
The buffer overflow test case is base 64 coded below (I did not minimize it since it is short anyway):