-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Adding Ruby Support into OSS-Fuzz via Ruzzy #12034
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
AdvenamTacet is integrating a new project: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it is almost there!
|
||
# The MAKE variable allows overwriting the make command at runtime. This forces the | ||
# Ruby C extension to respect ENV variables when compiling, like CC, CFLAGS, etc. | ||
ENV MAKE="make --environment-overrides V=1" |
There was a problem hiding this comment.
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 we're using some default CFLAGS and CXXFLAGS from OSS-Fuzz at this point so you might want to set them to nothing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe those variables are set.
https://github.com/google/oss-fuzz/blob/master/infra/base-images/base-clang/Dockerfile#L65-L67
If I should not depend on those, I should set them myself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My concern isn't that depending on them is bad, my concern is that these are "optimized' for fuzzing, but are not optimal for code that is being used to fuzz. E.g. "-O1".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can change it, but it also works as is and is tested that way. I will look into it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any reason to not use -O3
, initial tests didn't show any problem. If I confirm Tomorrow that new flags work same with all projects I have on disc, I will update that file.
#!/usr/bin/env bash | ||
|
||
fuzz_target=$(basename "$1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add "-e" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm testing it, I believe so.
@@ -0,0 +1,26 @@ | |||
#!/bin/bash | |||
# Copyright 2021 Google LLC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: 2024
@@ -0,0 +1,13 @@ | |||
#!/usr/bin/env bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this needs the copyright and license header.
projects/ox-ruby/build.sh
Outdated
@@ -0,0 +1,27 @@ | |||
#!/bin/bash -eu | |||
# Copyright 2016 Google Inc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: 2024 Google LLC
@@ -0,0 +1,12 @@ | |||
homepage: "https://github.com/ohler55/ox" | |||
language: ruby | |||
primary_contact: "[email protected]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not add your own contact info?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither @mschwager nor I are contributors to the project. We used it as a demonstration of Ruzzy's easiness of use and PoC, not with a goal to upstream it. That said, having an example project might be beneficial, so removing it from the Pull Request may not be ideal. However, managing those reports isn't something we aim to undertake. What's the best course of action here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries!
Can both of you sign the CLA? |
/gcbrun trial_build.py ruby --fuzzing-engines libfuzzer --sanitizers coverage |
I fixed an issue with the testing infra. Let's try agian. |
/gcbrun trial_build.py ruby --fuzzing-engines libfuzzer --sanitizers coverage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I implemented some issues, the rest I should do now. I will also resolve CLA issue.
|
||
# The MAKE variable allows overwriting the make command at runtime. This forces the | ||
# Ruby C extension to respect ENV variables when compiling, like CC, CFLAGS, etc. | ||
ENV MAKE="make --environment-overrides V=1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe those variables are set.
https://github.com/google/oss-fuzz/blob/master/infra/base-images/base-clang/Dockerfile#L65-L67
If I should not depend on those, I should set them myself.
#!/usr/bin/env bash | ||
|
||
fuzz_target=$(basename "$1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm testing it, I believe so.
@@ -87,6 +88,15 @@ RUN wget https://repo1.maven.org/maven2/org/jacoco/org.jacoco.cli/0.8.7/org.jaco | |||
COPY install_javascript.sh / | |||
RUN /install_javascript.sh && rm /install_javascript.sh | |||
|
|||
# Copy built ruby and ruzzy from builder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
139M /usr/local/rvm
140M /install/ruzzy
If it's important, I can try to reduce it (but without promises).
Can you add ruby to LANGUAGES in infra/constants.py please? |
Can we do this? Really excited to get this landed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your patience, I will sit to it today and clear remaining requests. Thank you for your feedback and help!
|
||
# The MAKE variable allows overwriting the make command at runtime. This forces the | ||
# Ruby C extension to respect ENV variables when compiling, like CC, CFLAGS, etc. | ||
ENV MAKE="make --environment-overrides V=1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can change it, but it also works as is and is tested that way. I will look into it.
@@ -0,0 +1,12 @@ | |||
homepage: "https://github.com/ohler55/ox" | |||
language: ruby | |||
primary_contact: "[email protected]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither @mschwager nor I are contributors to the project. We used it as a demonstration of Ruzzy's easiness of use and PoC, not with a goal to upstream it. That said, having an example project might be beneficial, so removing it from the Pull Request may not be ideal. However, managing those reports isn't something we aim to undertake. What's the best course of action here?
This commit brings Ruzzy, a coverage-guided fuzzer developed for pure Ruby code and Ruby C extensions, into the OSS-Fuzz project. This addition effectively integrates Ruby fuzzing support into OSS-Fuzz. Similar to Atheris, a Python fuzzer, Ruzzy uses libFuzzer for coverage instrumentation and the fuzzing engine. It offers support for AddressSanitizer and UndefinedBehaviorSanitizer, particularly when fuzzing C extensions. While the current code is functional, additional refinement and bug fixes may be required to ensure reliability. You can find further discussion about this development in issue: google#11967 Co-authored-by: mschwager <[email protected]>
This commit adds the 'Ox' project into the repository as a demonstration of how projects written in Ruby can be integrated with Ruzzy within the OSS-Fuzz framework. The Ox project acts as a straightforward illustrative example, demonstrating the steps to set up, configure, and utilize Ruzzy for fuzzing Ruby code. The harnesses within it are distinctly simplistic, solely to exemplify the interaction between Ruzzy and OSS-Fuzz. The primary aim is to provide a clear comprehension of how to employ Ruzzy with individual Ruby projects in the OSS-Fuzz environment. 'Ox' repository: https://github.com/ohler55/ox
I'm happy to report that I solved CLA failure, but I had to force-push to accomplish that (squashed all changed while doing that). I will test installing rvm from apt
and if it works, I will push that change. I don't see anything else I should do before merge, let me know if I missed something. One more time, thank you for your help! |
/gcbrun trial_build.py ruby --fuzzing-engines libfuzzer --sanitizers coverage |
@@ -87,6 +88,15 @@ RUN wget https://repo1.maven.org/maven2/org/jacoco/org.jacoco.cli/0.8.7/org.jaco | |||
COPY install_javascript.sh / | |||
RUN /install_javascript.sh && rm /install_javascript.sh | |||
|
|||
# Copy built ruby and ruzzy from builder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eh this is a little big. We can have it for now, but if you can do any cheap fixes it would help.
(size makes things harder for CIFuzz which needs to pull images quickly and has limited disk space.
] | ||
LANGUAGES_WITH_COVERAGE_SUPPORT = [ | ||
'c', 'c++', 'go', 'jvm', 'python', 'rust', 'swift', 'javascript' | ||
'c', 'c++', 'go', 'jvm', 'python', 'rust', 'swift', 'javascript', 'ruby' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we actually have coverage support for Ruby?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It's not perfect at the moment for pure Ruby, but it works and soon should be fixed (to work well). Works well with C extensions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to add a bit more context here: Ruby coverage support is currently okay, but not ideal. But we're also working with the upstream Ruby language maintainers to improve their stdlib coverage functionality: https://bugs.ruby-lang.org/issues/20448. This will improve Ruzzy's coverage gathering support significantly 👍
@@ -0,0 +1,12 @@ | |||
homepage: "https://github.com/ohler55/ox" | |||
language: ruby | |||
primary_contact: "[email protected]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries!
I think the integration is still broken. I'm going to try to fix it when i gt to the office. I'm going to use my own email for the ruby project for now so no one gets spurious emails. |
This is a follow-up to the discussions held during our Monthly Fuzzing Collaboration meetings and directly relates to issue #11967.
This Pull Request integrates Ruzzy support for Ruby fuzzing into OSS-Fuzz. Ruzzy is a coverage-guided fuzzer for pure Ruby code and Ruby C extensions, developed by Matt (@mschwager) at Trail of Bits. More information on Ruzzy can be found in the blog post titled "Introducing Ruzzy, a coverage-guided Ruby fuzzer".
The first commit of this PR integrates Ruby support into the OSS-Fuzz project via Ruzzy, while the second one includes the Ox project as an example of its usage.
The first commit introduces changes in the infra directory, most notably by adding the base-builder-ruby docker and the ruby install script.
Two scripts, ruzzy-build and ruzzy, have been added to base-builder-ruby and base-runner respectively. The former creates scripts that start harnesses with the latter one, and the latter is simply a wrapper for ruby with LD_PRELOAD.
In order to prevent the duplication of many gigabytes of data, we use separate installation directories for RubyGem. Technically, Ruzzy can be installed in the default directory without any performance disadvantage, but having a separate directory may facilitate troubleshooting.
This implementation was arrived at through testing a few ideas. If you have suggestions for further improvements, please let me know. I am currently addressing my concerns in the related issue.
Using the provided scripts isn't necessary but it does simplify the process. Installation directories are set using environment variables in the Dockerfiles, making it transparent for users.
The second commit simply adds a project to illustrate how straightforward the integration process is. You can test it using the standard helper commands.
Fixes: #11967
Co-authored-by: mschwager [email protected]