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

OSS-Fuzz integration #2888

Merged
merged 1 commit into from
Jun 20, 2024
Merged

OSS-Fuzz integration #2888

merged 1 commit into from
Jun 20, 2024

Conversation

tyler92
Copy link
Contributor

@tyler92 tyler92 commented Jun 18, 2024

As we discussed in #2887, we are adding Boost.Beast to OSS-Fuzz. I plan to proceed with several steps and would appreciate your help. The plan is as follows

  • add fuzzing targets (*.cpp files with LLVMFuzzerTestOneInput entry point)
  • add fuzzing seed (or initial corpus). This consists of a set of files grouped by target. Typically, these are stored as .zip or .tar files. In our case, I can create three files: http_request_fuzzer_seed_and_corpus.zip, http_response_fuzzer_seed_and_corpus.zip, and websocket_server_fuzzer_seed_and_corpus.zip. OSS-Fuzz expects this format, but we can store these files in our Git repository in any way that suits us. For example, Boost.URL uses a single tar file with directories inside.
  • add CMakeLists.txt and/or Jamfile. The specifics depend on who will build the fuzzers manually. We can create a CMakeLists.txt or Jamfile that simply compiles fuzzers, or we can implement more complex scenarios similar to what we have done for Boost.URL. I would be glad to receive your advice on this matter.
  • add fuzzers to CI. According to the official documentation, we need to create three files in .clusterfuzzlite directory and create a workflows file. However, @ashtum mentioned that another approach might be used. An example of clusterfuzzlite usage: link.

Copy link

codecov bot commented Jun 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.23%. Comparing base (e7f4919) to head (35f9e9c).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #2888   +/-   ##
========================================
  Coverage    93.23%   93.23%           
========================================
  Files          177      177           
  Lines        13675    13675           
========================================
  Hits         12750    12750           
  Misses         925      925           

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7f4919...35f9e9c. Read the comment docs.

@tyler92
Copy link
Contributor Author

tyler92 commented Jun 18, 2024

I added a draft version for .clusterfuzzlite directory. Obviously the Dockerfile has to be updated for the CI environment (instead of git clone), but it can be used as a starting point. Please let me know if it makes sense and we can continue with this approach.

Here is some information about how to test it locally: https://google.github.io/clusterfuzzlite/build-integration/#testing-locally

python infra/helper.py build_image --external $PATH_TO_PROJECT
python infra/helper.py build_fuzzers --external $PATH_TO_PROJECT --sanitizer address

# OK
python infra/helper.py run_fuzzer --external --corpus-dir /tmp/corpus $PATH_TO_PROJECT http_request_fuzzer

# Will fail due to https://github.com/boostorg/beast/pull/2881
python infra/helper.py run_fuzzer --external --corpus-dir /tmp/corpus $PATH_TO_PROJECT websocket_server_fuzzer

@ashtum
Copy link
Collaborator

ashtum commented Jun 19, 2024

@tyler92 If I understand correctly, we don't need to add the .clusterfuzzlite directory to Beast if we decide to build and run fuzzer targets ourselves, as we do for Boost.URL. Therefore, the contents of the .clusterfuzzlite directory belong to projects/boost-beast directory in oss-fuzz repository. Is that correct?

@tyler92
Copy link
Contributor Author

tyler92 commented Jun 19, 2024

@tyler92 If I understand correctly, we don't need to add the .clusterfuzzlite directory to Beast if we decide to build and run fuzzer targets ourselves, as we do for Boost.URL. Therefore, the contents of the .clusterfuzzlite directory belong to projects/boost-beast directory in oss-fuzz repository. Is that correct?

Yes, you are right. There are two options:

  1. Using integration ClusterFuzzLite which requires this directory.
  2. Building and running fuzzers manually. In this case, the content of .clusterfuzzlite is moved to oss-fuzz repository. I can do it after we finish with this PR. Beast needs to contain only the fuzzer targets and corpus (which is already done).

The first option should be easier, but the second one is probably more flexible and requires additional effort. Let me know which is better for Beast, and I will move the ClusterFuzzLite-related files accordingly. I would probably ask for your help if we choose the second option because I'm not very familiar with the Beast's build system.

@tyler92
Copy link
Contributor Author

tyler92 commented Jun 20, 2024

@ashtum Should I add a CMakeLists.txt similar to Boost.URL?

@ashtum
Copy link
Collaborator

ashtum commented Jun 20, 2024

@ashtum Should I add a CMakeLists.txt similar to Boost.URL?

There is also https://github.com/boostorg/json/blob/develop/fuzzing/Jamfile, which looks solid. I'm currently experimenting with these and testing them in CI. The problem with using ClusterFuzzLite is that it requires an additional repository to save corpus files and cannot use the GHA cache feature.

@tyler92
Copy link
Contributor Author

tyler92 commented Jun 20, 2024

OK, just for reference - PR for oss-fuzz google/oss-fuzz#12109 which is WIP until this PR is merged (it requires fuzzing targets and seeds from Beast's repo).

@ashtum
Copy link
Collaborator

ashtum commented Jun 20, 2024

OK, just for reference - PR for oss-fuzz google/oss-fuzz#12109 which is WIP until this PR is merged (it requires fuzzing targets and seeds from Beast's repo).

It looks good thank you. I think we might want to remove this line because the Boost repository is checked out at master:
https://github.com/google/oss-fuzz/pull/12109/files#diff-2d7de9a413e01b701ddb0903c9c94831e32b6c2a47c0b9f9a2d5a433752310b6R20

RUN git -C libs/beast checkout develop

In the next step, please remove the .clusterfuzzlite directory from this PR and force push a single commit that adds the test/fuzz directory. I want to merge your PR so you can continue your work on google/oss-fuzz#12109. I'll setup the CI and build scripts in a separate commit.

@tyler92
Copy link
Contributor Author

tyler92 commented Jun 20, 2024

In the next step, please remove the .clusterfuzzlite directory from this PR and force push a single commit that adds the test/fuzz directory. I want to merge your PR so you can continue your work on google/oss-fuzz#12109. I'll setup the CI and build scripts in a separate commit.

Done

It looks good thank you. I think we might want to remove this line because the Boost repository is checked out at master: https://github.com/google/oss-fuzz/pull/12109/files#diff-2d7de9a413e01b701ddb0903c9c94831e32b6c2a47c0b9f9a2d5a433752310b6R20

RUN git -C libs/beast checkout develop

OK, sure, let me do that after we merge this PR to let oss-fuzz checks pass. BTW: do I need to add you to auto_ccs in project.yaml there?

@ashtum
Copy link
Collaborator

ashtum commented Jun 20, 2024

Sorry, I forgot to mention that there is no need for the _fuzzer suffix for cpp files, as they are all in the test/fuzz directory. Please remember to update seeds.tar afterward.

OK, sure, let me do that after we merge this PR to let oss-fuzz checks pass. BTW: do I need to add you to auto_ccs in project.yaml there?

Yes please, [email protected].

@tyler92
Copy link
Contributor Author

tyler92 commented Jun 20, 2024

Sorry, I forgot to mention that there is no need for the _fuzzer suffix for cpp files, as they are all in the test/fuzz directory. Please remember to update seeds.tar afterward.

Fixed

@ashtum ashtum merged commit 1b87492 into boostorg:develop Jun 20, 2024
1 check was pending
@tyler92
Copy link
Contributor Author

tyler92 commented Jun 21, 2024

OK, just for reference - PR for oss-fuzz google/oss-fuzz#12109 which is WIP until this PR is merged (it requires fuzzing targets and seeds from Beast's repo).

It looks good thank you. I think we might want to remove this line because the Boost repository is checked out at master: https://github.com/google/oss-fuzz/pull/12109/files#diff-2d7de9a413e01b701ddb0903c9c94831e32b6c2a47c0b9f9a2d5a433752310b6R20

RUN git -C libs/beast checkout develop

@ashtum Are you sure it's better to use master branch there? E.g. boost-json project has 'develop' branch and master for Boost. I don't know if this is correct, but probably it's better to use 'develop' for both Beast and Boost to detect issues as early as possible before they are merged to master. I might be not so familiar with the branch policy here, what's your opinion?

@ashtum
Copy link
Collaborator

ashtum commented Jun 21, 2024

@ashtum Are you sure it's better to use master branch there? E.g. boost-json project has 'develop' branch and master for Boost. I don't know if this is correct, but probably it's better to use 'develop' for both Beast and Boost to detect issues as early as possible before they are merged to master. I might be not so familiar with the branch policy here, what's your opinion?

There is a possibility that if we get a breaking change in one of the dependencies and update Beast in the develop branch, it will fail when trying to build it against Boost master. However, I would say this is a rare possibility and would be transient if it occurs. So you might want to keep it as it is (checking out the develop branch).

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.

None yet

2 participants