search

radareorg / sdb

Simple and fast string based key-value database with support for arrays and json
https://www.radare.org/
MIT License
218 stars 62 forks source link

Fix several bugs found with AFL #133

Closed EliaGeretto closed 7 years ago

EliaGeretto commented 7 years ago

This pull requested is a new version of #132, I applied the fixes suggested.

alvarofe commented 7 years ago

Could you add test for those issues to add them into the test suite?

EliaGeretto commented 7 years ago

I don't know if it's worth it. It's mainly malformed input, like parsing variables as JSON even if they are not actual JSON.

alvarofe commented 7 years ago

Well those crashes can appear in the future

EliaGeretto commented 7 years ago

Actually, my point is that, providing corrupted inputs, the outputs are quite random. Just changing implementation details, the tests will probably break without a valid reason. Then, the great majority is memory errors, found with an address sanitizer, so they would not be caught by your test suite anyway. I will upload the tests produced by AFL here so you can check them out if you want. afl_test_suite.zip

radare commented 7 years ago

Sorry for the delay, i have just reviewed your PR, it looks good! just added two little comments and i'll love to merge as soon as you fixed them.

For the testsuite, yes, i think we can include those fuzzed tests and run it with asan or valgrind to ensure nothing regresses. those should be commited into ./test/fuzz and i would suggest you to make a separate PR for those.

Thanks!

alvarofe commented 7 years ago

Even if travis doesn't run the test with ASAN, I do locally from time to time, so having those tests are good to avoid regressions regarding crashes, not about checking output.

radare commented 7 years ago

Tired of waiting. i'll do the fixes now

radare commented 7 years ago

everything was merged and fuzzsuite pushed too. thanks for the work! we'll PR an updated sdb for r2 tonight to see if travis have any complain

EliaGeretto commented 7 years ago

Sorry for the delay, national holiday in the Netherlands. Thanks for doing the last fixes.

radare commented 7 years ago

All is good :) thanks to you for the pr

On 28 Apr 2017, at 10:53, Elia Geretto notifications@github.com wrote:

Sorry for the delay, national holiday in the Netherlands. Thanks for doing the last fixes.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.