Add a simple fuzz test for jsoncpp. #610
Conversation
|
BTW, the only bit of information I need to actually propose it for oss-fuzz is a google-associated email address. See https://github.com/google/oss-fuzz#accepting-new-projects |
|
Would my @gmail address suffice? Then you could set this up? If you'd rather use your own email address for convenience, that's fine too. Or use both if possible. I can't say I completely understand what's going on here, but I don't object. |
|
Yeah, a gmail address works fine. The one you commit with / in AUTHORS, right?
The idea is supposed to be that we let oss-fuzz pass in (potentially garbage) data into jsoncpp and see if it triggers accesses to uninitialized memory or other problems that might indicate a security issue. So this test should just accept that data and parse it. The dumb trick with the hashing to get flags is just to increase the possible code coverage. |
Internal (YouTube) bug tracking adding this: b/37561473 See also: open-source-parsers/jsoncpp#610
Internal (YouTube) bug tracking adding this: b/37561473 See also: open-source-parsers/jsoncpp#610
src/test_lib_json/main.cpp
Outdated
| @@ -1,9 +1,10 @@ | |||
| // Copyright 2007-2010 Baptiste Lepilleur | |||
There was a problem hiding this comment.
I hate to remove Baptiste's name anywhere. He really was the primary author. Could we way something like "BL and the JsonCpp authors"?
There was a problem hiding this comment.
Sorry, missed this because I apparently am having trouble getting updates from github (they go to my personal email instead of work email.)
The previous thing I did was replace with The JsonCpp authors. I actually missed a bunch of files by accident. Want I should redo it for "Baptiste Lepilleur and the JsonCpp Authors?" globally in a separate PR?
There was a problem hiding this comment.
Sorry, but I wouldn't feel right approving a commit that removes Baptiste's name from any file.
There was a problem hiding this comment.
This has been fixed. (idk how github code review works, I guess I have to respond to mark this as done, since it still has the orange blob saying work needs to be done?)
…ere it was missing. Requested/noticed in open-source-parsers#610, and a followup to open-source-parsers#607 .
This brings in the change requested in PR open-source-parsers#610 and implemented in PR open-source-parsers#637.
The idea is to then add it to the google oss-fuzz project and get jsoncpp
regularly tested for bugs.
https://github.com/google/oss-fuzz/
I've followed the instructions at:
https://github.com/google/oss-fuzz/blob/master/docs/ideal_integration.md
Tested using a simple oss-fuzz setup and with the modified jsoncpp unit tests
(cmake . && make).
I don't really know git, so also I've royally screwed up the commit history, and
artificially flattened it into this one commit. :)
|
I think I'm confused. I thought this was merged, but I guess it was only "approved". Is there anything that needs to be done on my end? (Sorry, new to git/github...) |
|
I don't remember why it wasn't merged back then. Maybe tests were not passing? Recently we started having problems with TravisCI, which makes a merge even harder. And I want to be sure that the runtime has not gotten worse. But most important, we now use Meson instead of Cmake. |
|
I'm not sure about meson. It looks like the cmake files are still here, and It merged cleanly. Is the plan to get rid of cmake eventually? I can't set up meson / test any changes there because my version of Ubuntu is too old. (Only has Ninja 1.3, Meson requires Ninja 1.6?) so I'd prefer to keep using cmake if that's possible, and let you update the meson config. |
|
Ok, as long as you're confident in your changes. This is only available to cmake users, and we no longer test cmake in TravisCI. (TravisCI is kinda broken right now anyway, because of python3.) |
|
Unmerged. This was failing in TravisCI, and it was merged when TravisCI itself was having problems (a transition to new O/S). This needs to be rebased, and then the tests need to pass. |
|
Any progress on OSS-Fuzz integration? Re: google/oss-fuzz#575 //cc @kcc |
| extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { | ||
| Json::CharReaderBuilder builder; | ||
|
|
||
| uint32_t hash_settings = JenkinsOneAtATimeHash(data, size); |
There was a problem hiding this comment.
Rather than hashing the input, it's simpler to just use the first byte or two for configuring options. We can early return 0 if size < REQUIRED_BYTES_FOR_CONFIG, and the fuzzer will figure out not to follow this edge pretty quickly.
|
No progress. I've been swamped. (And I also took like a month off, which didn't help.) |
Internal (YouTube) bug tracking adding this: b/37561473 See also: open-source-parsers/jsoncpp#610
The idea is to then add it to the google oss-fuzz project and get jsoncpp regularly tested for bugs.
I've followed the instructions at Ideal Integration
Tested using a simple oss-fuzz setup and with the modified jsoncpp unit tests (
cmake . && make).