-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Fuzzing results #431
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
could you describe result? |
@ya1gaurav, please handle these cases for us. If this is legit (i.e. not spam) then please put these tests on a branch, and if you have time, try to fix them. |
Sure, I will check these once i get enough information. |
The findings are legitimate, I simply didn't want to post them publicly. What is the best way to get them to you privately? |
I think if you post public, many people will help to fix possible bugs. Sharing and getting reviews is always good & this is Open Source - means open for all people :) |
That's a pretty naive view in my opinion. The truth is that jsoncpp is in many commercial products and some of these crashes could be exploitable -- at a minimum, they would cause DoS conditions. By posting them publicly I would be enabling exploitation of said software. I'd prefer a more responsible approach personally. |
Seems like spam to me. But if not, the way to share privately is to make your fork private. (I think you can do that, but if not, then just create a new private repo and push your changes there.) GitHub allows up to 5 private repos free of charge, so there is no downside. Then add @ya1gaurav and me as collaborators. |
I'm just trying to report crash conditions, not spam. I find the difficulty in doing so pretty offensive. I'm not going to create a fork and fix the problems for you guys -- I was trying to be helpful by reporting them responsibly. |
A gist can be private too. I'll let @ya1gaurav handle this though. I don't have time right now, sorry. |
@netsurf916 Data inside [jsoncpp/raw/crashes] if try to parse using jsoncpp crashes, you mean to say this ? |
I think these are deeply nested, so give below message: |
Yeah, the files in that directory are the cases that cause the crashes. It's reasonable they are all related to only a few actual bugs also. Fixing one may fix them all. If you are using recursion, then it would make sense to impose some limits to prevent crashing since json parsing is generally an exposed feature in some form or another. |
All cases report -
As your test case does not handle exception, so it will crash anyway. |
The test app I adapted the fuzzing app from also does not use exception handling (or I missed it), so it's not indicative of expected usage. Exception handling is not always available -- depending on the system and coding practices. |
Exception occured with below message:
It means such a json input is restricted by jsoncpp. |
You seem to be referring only to stackoverflows. See #56, #88, and #166. JsonCpp used to crash on such input, but now it issues exceptions. That is how the library works. Aside from possibly improving documentation, I don't think any action needs to be taken. @netsurf916, thanks for trying the fuzzing. It definitely increases our confidence in the library. @ya1gaurav, thanks for following up. |
Please contact me for 34 crash cases identified during fuzz testing with afl.
The text was updated successfully, but these errors were encountered: