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.

Sign up for GitHub

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

Jump to bottom

Initial commit of a fuzzer for Meshtastic #5790

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Initial commit of a fuzzer for Meshtastic #5790

wants to merge 17 commits into from
+671 −7

Conversation

esev
Copy link
Contributor

@esev esev commented Jan 8, 2025 •
edited
Loading

This PR implements a fuzzer using the ClusterFuzzLite (OSS-Fuzz) framework. More background about the steps required to generate a fuzzer can be found in the Build Integration Guide for ClusterFuzzLite.

Some things to note:

  • ClusterFuzzLite requires the binary to be built with specific compiler options and use the clang/llvm tool chain. I've implemented this using the buildroot variant, which provides a convenient way to override the build tools used for PlatformIO.
  • A FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION CPP define is used for enabling features needed for fuzzing. I've tried to make as few changes as necessary to keep the code mostly the same while running under the fuzzer.
  • .__cxa_exception_type() is not implemented in clang. I've implemented the solution in https://stackoverflow.com/a/47164539/4613163 within PortduinoGlue.cpp.
  • clang also produces an error when a type is being narrowed (a uint64 being assigned to a uint32). A static_cast<uint32_t>(value) silences this error.
  • I've only gotten the fuzzer working with the Address Sanitizer. I've also tried the Memory Sanitizer, but it'll require recompiling a few more dependencies using clang and the fuzzer provided build flags.

I've included a README and tried to document things so it is clear how the fuzzer works and what all the files are used for. If anything is unclear, please ask and I'll improve the comments/documentation in this PR.

@esev esev changed the title Initial commit of a fuzzer for Meshtastic. Initial commit of a fuzzer for Meshtastic Jan 8, 2025
@fifieldt
Copy link
Contributor

fifieldt commented Jan 8, 2025

This is advanced stuff, we're very lucky to have you. Thanks for the huge effort.

@esev
Copy link
Contributor Author

esev commented Jan 8, 2025 •
edited
Loading

Thank you @fifieldt ! To be fair this was a lot of guessing & trial and error to get it going too. :)

I implemented a limit on the TypedQueue class used by the native/portduino build. Using 0 still allows the original behavior of a limitless queue. Let me know if you'd prefer that change to be guarded by FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION instead.

I could be reading the code wrong, but it looks like there is no settingsMap[maxtophone] limit configured by default unless the yaml config file has a "General" section. So I also set settingsMap[maxtophone] = 5 in the fuzzer. This is also the reason I kept a 0 queue size as limitless; in case there is no default I wasn't sure how the firmware would behave when numFree always returned 0. Perhaps it would be better to assert(maxElements > 0); but I'd prefer if that could be worked out in another PR to keep this one scoped to changes needed for the fuzzer.

@esev
Copy link
Contributor Author

esev commented Jan 9, 2025 •
edited
Loading

Still to-do:

  • Have the fuzzer shut down cleanly on exit without crashing.
  • Get coverage report generation working.
  • Add Github actions to run the fuzzer on PRs and/or on a schedule for longer runs.

The fuzzer is functional for anyone who wants to run it locally. It just can't be run via an action until it shuts down cleanly. If this PR looks good I think it can be merged. I'll keep working on this and either add more commits to this PR or send new PRs separately for each of the to-do items.

Also, if you'd prefer, I can split those TypedQueue changes into a separate PR that is scoped just to that change only.

@fifieldt fifieldt force-pushed the cflite branch 2 times, most recently from 2f98807 to 73e1f09 Compare January 10, 2025 10:47
@esev
Copy link
Contributor Author

esev commented Jan 10, 2025

#5807 should resolve Have the fuzzer shut down cleanly on exit without crashing

esev added 17 commits January 11, 2025 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fifieldt fifieldt fifieldt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@esev @fifieldt