Skip to content
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

Setup for persistent fuzzing #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

quinox
Copy link
Contributor

@quinox quinox commented Nov 15, 2022

DO NOT MERGE AS-IS

There's a bomb in the code to show that the fuzzing is finding results. You might want to verify for yourself that the setup is doing what it's supposed to do + remove the bomb. Feel free to canibalize this PR to your heart's content.

Also: the docker build fails. Something to do with an older cmake? Setup works correctly even on Debian now.

What's this about

A setup for doing persistent mode fuzzing with AFL++ using shared memory instead of files as input. This gives superior speed and makes it easy to target different parts of the code.

I made it as easy as possible to add new fuzzing targets: copy a tiny file into fuzz-persistent/targets/ and you're good to go. There's magic in the Makefile to auto-generate targets, and the fuzz-helper.sh also contains magic to auto-find stuff. Compilation is a bit slow because it adds all *.ccp files even those you don't need but on a modern machine it shouldn't be much of a problem. The alternative is writing out a CMake entry for every fuzzing target which sounds annoying.

(I foresee it's possible to build a hybrid solution where you use the autogenerated setup unless you configure something specific in the CMakeLists.txt.)

How to use

cd fuzz-persistent/
./fuzz-helper.sh

See also fuzz-persistent/README.md.

@quinox quinox force-pushed the feat-persistent-fuzzing branch from 8e3120e to d8cd4db Compare February 2, 2023 11:10
@quinox quinox force-pushed the feat-persistent-fuzzing branch 3 times, most recently from 9f060f7 to ebff4a1 Compare March 12, 2023 14:12
@quinox
Copy link
Contributor Author

quinox commented Mar 12, 2023

All is done, I also fixed the failing Docker part: on older CMakes it will now skip setting up the fuzzing targets

EDIT: I implemented a fall back to a regular expression to extract the file stem for older CMakes.

@quinox quinox force-pushed the feat-persistent-fuzzing branch 2 times, most recently from 5e15ea8 to b43c98c Compare March 13, 2023 21:49
@quinox quinox force-pushed the feat-persistent-fuzzing branch from b43c98c to 47ce529 Compare April 9, 2023 08:32
@quinox quinox force-pushed the feat-persistent-fuzzing branch 4 times, most recently from fb7c08e to cd5834a Compare May 10, 2024 09:37
@quinox
Copy link
Contributor Author

quinox commented May 10, 2024

I rebased the branch on master, everything is still in working order. I added one more example to determine a more realistic speed, namely MqttPacket::bufferToMqttPackets.

Testcase Speed (single core)
fuzz_cirbuf__write 47.1k/sec
fuzz_mqttpacket__bufferToMqttPackets 8779/sec
fuzz_utils__base64Encode 44.2k/sec

The startup screen:
start

Testing MqttPacket::bufferToMqttPackets against vanilla master. These crashes might be legit if I wrote the testcase correctly:
buffertoMqttPackets

Testing base64Encode (with a bomb added on purpose):
base64Encode

@halfgaar
Copy link
Owner

I'm having trouble getting it to work, with errors like use of undeclared identifier '__AFL_FUZZ_TESTCASE_LEN'. It can't find many AFL things.

Do you have the ability to run the saved test case outside afl and see the crash (in a debugger)? If you do ulimit -c unlimited, and potentially echo core >/proc/sys/kernel/core_pattern to disable a core handler, you can start gdb afterwards, with core file like gdb flashmq core.

@halfgaar
Copy link
Owner

BTW, you may have a bug in your test. You need to catch ProtocolError. In fact, any exception will just cause a client disconnect, so is not harmful. If you don't catch them, they result in a signal ABORT.

@quinox
Copy link
Contributor Author

quinox commented May 10, 2024

The __AFL_FUZZ_INIT and friends are magical macros that are replaced by the AFL compiler itself. I suppose it can happen when AFL++ is not recent enough maybe? It worked with AFL++ from their git repo when I opened this PR, end of 2022. I'm not entirely sure but based on my backups it was probably afl-cc++4.04a using clang 14.

Right now I'm using a freshly baked version:

quinox@gofu ~> ~/tmp/AFLplusplus/afl-clang-lto --version
afl-cc++4.21a by Michal Zalewski, Laszlo Szekeres, Marc Heuse - mode: LLVM-LTO-PCGUARD
clang version 17.0.6
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/lib/llvm/17/bin
Configuration file: /etc/clang/x86_64-pc-linux-gnu-clang.cfg

Their documentation can be found here: README.persistent_mode

You need to catch ProtocolError.

Thanks, that fixed the majority of the findings. It took 10 minutes of fuzzing to a new set of crashes but it's only reproducible on a Cirbuf size of 1024 and the crash case is 12k which doesn't make sense. I'll look into it later.

@halfgaar
Copy link
Owner

I did find those macros in the AFL source. I even tried setting an include path, but just couldn't get it to work.

Thanks, that fixed the majority of the findings. It took 10 minutes of fuzzing to a new set of crashes but it's only reproducible on a Cirbuf size of 1024 and the crash case is 12k which doesn't make sense. I'll look into it later.

You're also missing a call to ensureFreeSpace() so you're overwriting the boundry. The crash you're getting is likely an ABORT on an assert. It would be nice if AFL fuzz showed the signal number of the crash in the GUI.

There are some other considerations. I wrote a simple test case using simple brute-force fuzzing to illustrate them: Add test to briefly perform some fuzzing on parsing packets

Also, of importance is Add assert to document initial size restriction on buffer.

@quinox quinox force-pushed the feat-persistent-fuzzing branch from cd5834a to da3314b Compare May 11, 2024 07:00
@quinox
Copy link
Contributor Author

quinox commented May 11, 2024

I did find those macros in the AFL source. I even tried setting an include path, but just couldn't get it to work.

The fuzz-helper.sh should force either afl-clang-lto (fastest) or afl-clang-fast to be used, which should be enough. In their documentation they have a section about using other compilers if you want to:

#ifndef __AFL_FUZZ_TESTCASE_LEN
  ssize_t fuzz_len;
  #define __AFL_FUZZ_TESTCASE_LEN fuzz_len
  unsigned char fuzz_buf[1024000];
  #define __AFL_FUZZ_TESTCASE_BUF fuzz_buf
  #define __AFL_FUZZ_INIT() void sync(void);
  #define __AFL_LOOP(x) ((fuzz_len = read(0, fuzz_buf, sizeof(fuzz_buf))) > 0 ? 1 : 0)
  #define __AFL_INIT() sync()
#endif

You're also missing a call to ensureFreeSpace() so you're overwriting the boundry.

After adding this the last crash disappeared.

It would be nice if AFL fuzz showed the signal number of the crash in the GUI.

For sure. The data that crashes the program is great, but also logging the backtrace etc. would be most helpful.

There are some other considerations.

Yeah, at this point I'm out of my depth writing the fuzz tests themselves. For example calling packet.handle() on the elements found in my vector<MqttPacket>parsedPackets looked like a great way to improve the scope of the fuzzing but led to an unstable testcase. Yesterday I used gdb to figure out I needed to call ThreadGlobals::assignSettings(&settings); for the testcase to work at all and I was quite happy with finding that solution, but that was easy because it crashed consistently: tracking down instability issues is not as easy for me.

For the most speed you also want to do as much initialization of your own code before the call to __AFL_INIT(), which requires knowledge of FlashMQ internals. If you manage to get this setup working and are serious about using its powers you also might need to do some more refactoring: fe. normally data gets into the client by calling readFdIntoBuffer but we don't have an Fd in this setup, and readbuf is a protected member. An obvious workaround which I resorted to was creating a Client::readBufIntoBuffer (hidden for real builds by using a #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION) that would modify readbuf for me, but this way I am bypassing quite a bit of code and the testcase is gaining more and more responsibility for flow control which is not ideal: you want to test FlashMQ itself, not your own testing code.

I limit the scope of this PR to "POC of real fast fuzzing using persistent mode". I'm happy to help you figure out why your AFL++ isn't correctly substituting its own macros, most of the other work is either knowing FlashMQ and/or making decisions about how to structure the codebase: I'll leave that to you.

@halfgaar
Copy link
Owner

I do want to merge this setup in, that's for sure. The current fuzzing harness that writes to the fd has issues anyway. Not about the fd, but for instance: it enables websocket mode by the filename, but that filename is changed by AFL to something else.

That fd/buf issue is probably (somewhat) easily solved by using something like a socketpair; it's like pipe but then you can read/write on both ends. You can give one socket to the client, and write the AFL buf into the other end. The sockets should be non-blocking, so you don't need and event loop and can just call readFdIntoBuffer().

@quinox quinox mentioned this pull request Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants