halfgaar / FlashMQ

FlashMQ is a fast light-weight MQTT broker/server, designed to take good advantage of multi-CPU environments
https://www.flashmq.org/
Open Software License 3.0
185 stars 24 forks source link

Faster fuzzing #11

Closed quinox closed 3 months ago

quinox commented 1 year ago

On my system using AFLplusplus, for a single core:

quinox commented 1 year ago

Let's consider this a WIP: there are more changes needed to enable fuzzing once more, right now the binary always terminates with File '/var/log/flashmq/flashmq.log' is there, but not writable for me unless I also specify a custom config.

After modifying the setup a bit more to make it pass the sanity checks I now get for a single core:

quinox commented 1 year ago

It's now ready to be merged: I've rebased and updated my branch with all the changes needed to bypass the sanity checks.

Note that I've used #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION in 1 spot to disable a threading assert: when --fuzz-file is given threading is not used, so this assert seems undesirable. That #ifdef is supported by AFL, libfuzzer etc.

There are further optimizations possible: the documentation describes using shared memory instead of files for a 10x speedup (ALF++: Persist Mode). Using separate entry points that call specific functions directly might also mean no more workaround are needed to bypass the sanity checks, and easier to fuzz different parts of the code / different modes (such as the if (fuzzWebsockets && strContains(fuzzFilePathLower, "upgrade")) logic). I might look into that at some point.

halfgaar commented 1 year ago

Great stuff, thanks!

I do have comments though. Execution speed was made very slow by having a storage_dir. You can leave it out of the temporary config file; the default is none. Perhaps you have a system-side /etc/flashmq/flashmq.conf with a storage dir defined, leading you to believe it had a default one? The binary does actually have a default config of /etc/flashmq/flashmq.conf. I once fell in that trap, having 'mystery' settings while developing...

Also, it was an issue that that storage dir was not in $OUTPUTDIR. At least the original AFL documentation states to put the output on non-SSD, because it can wear it down. But even now that is no longer an issue, isn't it better to store that runner dir in $OUTPUTDIR too, instead of $FLASHMQ_SRC/fuzzrun?

And a question: I see the detection of the compiler doesn't consider afl-clang++. Is there a reason for that?

And about the entry points: yes, there are many improvements to be made. There could be separate targets in the Cmake file for that, that only throw data at IoWrapper for instance, one of the parts that benefits most from fuzzing. And I still want to look at https://github.com/aflnet/aflnet.

I also broke the helper script with an extra cd in build.sh, as part of building the manual pages. Perhaps pushd and popd would be better.

quinox commented 1 year ago

Thanks for the feedback.

I do have comments though. Execution speed was made very slow by having a storage_dir. You can leave it out of the temporary config file; the default is none.

Good to know, I indeed thought it was mandatory. I'll update the PR and leave it out.

And a question: I see the detection of the compiler doesn't consider afl-clang++. Is there a reason for that?

None in particular. I really wanted to use the speed of LTO and while working on that I found the fast one. I assume the speed of afl-clang++ is comparible to the speed of alf-g++.

And about the entry points: yes, there are many improvements to be made. There could be separate targets in the Cmake file for that, that only throw data at IoWrapper for instance, one of the parts that benefits most from fuzzing. And I still want to look at https://github.com/aflnet/aflnet.

This morning I managed to get the persistent-mode-with-shared-memory-input working, it runs at 60.000 execs per second on a single core when using the base64encoder as target, it blasted through it's iterations like there's no tomorrow. I programmed in a segfault (crash when length is precisely 13 bytes) and it managed to find it / make a testcase for it, so it's legit. (see hacky patch)

Before making a PR I wanted to fuzz something slightly more realistic so I tried making it fuzz the same logic as what now happens with the fuzzFile but that requires more fiddling. I'll see if fuzzing ioWrapper is easier to get going.

quinox commented 3 months ago

I'll close this PR in favor of #18: that one is superior in speed and flexibility.