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

infra: add support for netdriver fuzzing with honggfuzz #7351

Merged

Conversation

DavidKorczynski
Copy link
Collaborator

Adds an example with the mongoose webserver

Copy link
Contributor

@jonathanmetzman jonathanmetzman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Another exciting change!

@DavidKorczynski
Copy link
Collaborator Author

I think this could face some of the issues mentioned in #7347

@robertswiecki
Copy link
Contributor

Sorry for late response, I think this can be useful

/* HF NetDriver signature - if found within file, it means it's a NetDriver-based binary */
#define _HF_NETDRIVER_SIG "\x01_LIBHFUZZ_NETDRIVER_BINARY_SIGNATURE_\x02\xFF"

https://github.com/google/honggfuzz/blob/623d8bb620c0d8ca8ac67a0dea8ab4cf835ea706/honggfuzz.h#L56

@DavidKorczynski DavidKorczynski marked this pull request as ready for review March 3, 2022 13:59
@DavidKorczynski
Copy link
Collaborator Author

@jonathanmetzman -- you approved this but I wanted to double check if it's ready before merging it in?

Copy link
Contributor

@jonathanmetzman jonathanmetzman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah lets wait until I make some ClusterFuzz-side changes to merge this.

DavidKorczynski added a commit to DavidKorczynski/clusterfuzz that referenced this pull request Jul 20, 2022
Copy link
Collaborator

@oliverchang oliverchang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're kicking off a deployment for the ClusterFuzz instance, so this should hopefully be ready to go there soon.

projects/mongoose/build.sh Outdated Show resolved Hide resolved
projects/mongoose/build.sh Outdated Show resolved Hide resolved
infra/base-images/base-builder/compile_honggfuzz Outdated Show resolved Hide resolved
Copy link
Collaborator

@oliverchang oliverchang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@DavidKorczynski DavidKorczynski force-pushed the honggfuzz-netdriver-mongoose branch from ab3845b to 6833c6e Compare September 6, 2022 09:56
Copy link
Contributor

@jonathanmetzman jonathanmetzman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

infra/base-images/base-builder/compile_honggfuzz Outdated Show resolved Hide resolved
infra/base-images/base-runner/run_fuzzer Outdated Show resolved Hide resolved
HFND_FUZZING_ENTRY_FUNCTION(int argc, char **argv) {
struct mg_mgr mgr;
mg_mgr_init(&mgr);
mg_http_listen(&mgr, "http://0.0.0.0:8666", fn, &mgr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will have some issues when we have multiple fuzzers. Because we run bad_build_check on all of them at once but they can't use the same port.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, would you prefer to address this now? One solution is to scan the fuzzers and check if there is more than one fuzzer with netdriver settings, and, if so, avoid running the fuzzers in parallel.

Copy link
Contributor

@robertswiecki robertswiecki Sep 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if that's the case here, but hf netdriver uses net namespaces just for that. It should work with a single/same port per fuzzing thread.

https://github.com/google/honggfuzz/blob/60f01c26d9200a4a1058c35a8fc22441b63fa88c/libhfnetdriver/netdriver.c#L104-L124

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@jonathanmetzman
Copy link
Contributor

/gcbrun trial_build.py mongoose --sanitizer address coverage memory undefined --fuzzing-engine libfuzzer afl honggfuzz

@jonathanmetzman
Copy link
Contributor

Trial builds don't work here because they don't work on forked repos :-(

@DavidKorczynski
Copy link
Collaborator Author

Trial builds don't work here because they don't work on forked repos :-(

Do I need to do something to make this work, or how do you prefer to move forward?

@oliverchang
Copy link
Collaborator

Trial builds don't work here because they don't work on forked repos :-(

Do I need to do something to make this work, or how do you prefer to move forward?

The changes look relatively self contained, so let's merge.

Going forward, for infra-related PRs like these, can you create them off a branch on the main repo instead?

@oliverchang oliverchang merged commit dc5adbf into google:master Sep 12, 2022
@jonathanmetzman
Copy link
Contributor

Trial builds don't work here because they don't work on forked repos :-(

Do I need to do something to make this work, or how do you prefer to move forward?

The changes look relatively self contained, so let's merge.

Going forward, for infra-related PRs like these, can you create them off a branch on the main repo instead?

It shouldn't be too hard to support forks, I'm just avoiding implementing this to reduce bus factor: #8273

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.

4 participants