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

Integration with google/oss-fuzz for continuous fuzz testing #775

Open
nathaniel-brough opened this issue Nov 5, 2023 · 40 comments
Open

Comments

@nathaniel-brough
Copy link
Contributor

Hey Rhai Team,

I hope this message finds you well. I've been following along with Rhai for some time now, and I've really enjoyed using it in one of my side projects. I'd like to suggest and champion an effort to set up some basic fuzz-testing and combine it with google/oss-fuzz for continuous fuzzing. I'm fully aware that you are very busy and I don't want to overload your review/maintenance capacity by introducing too many new ideas. Is this a bad time to discuss potential security/reliability improvements?

If your not familiar with fuzzing or oss-fuzz I've included a few brief notes below.

Benefits of Fuzz-Testing

  • Dynamic Code Testing: Fuzz-testing challenges systems with unexpected data, aiming to identify vulnerabilities. It’s akin to an exhaustive stress-test for the code.
  • Detecting Hidden Vulnerabilities: It can uncover potential weaknesses that may not be evident in routine tests.
  • Continuous and Automated Testing: With tools like Google’s OSS-Fuzz, fuzz-testing can be automated, running continuously on distributed systems, ensuring daily resilience checks.

Google/oss-fuzz for Continuous Fuzzing

  • Automated Fuzzing: OSS-Fuzz undertakes comprehensive fuzz-testing daily on a distributed cluster.
  • Security Boost: It provides enhanced security measures, at zero cost for open-source projects.
  • Detailed Reporting: OSS-Fuzz offers exhaustive reports in case of detected anomalies, enabling effective action.

I’d be more than happy to lead the effort in integrating fuzz testing with Rhai and assist in any way required.

As a proof of concept I created a super simple minimal fuzz harness for the Rhai in #774

@schungx
Copy link
Collaborator

schungx commented Nov 6, 2023

Fuzzing is definitely welcome! I personally don't know much about setting one up and it is great that you offer to help.

Let me know what you need and I can set things up your way.

@nathaniel-brough
Copy link
Contributor Author

Cool, there is an application process for google/oss-fuzz. I'm pretty confident that this project will get accepted as it is quite popular.

A few notes that I should have included in my original message;

  • google/oss-fuzz will send you through an email link to a private bug-tracker whenever it finds a bug.
  • It'll provide the files needed to reproduce the bug and automatically categorize the bug e.g. (Security, Non-critical etc.)
  • You'll be able to view all the crash's and statistics from an online dashboard.

I can complete the application on your behalf. To do this I'll need the following from you;

  • A google/gmail email address. This is required to login into the bug-tracker/oss-fuzz dashboard. I can use a non-gmail/google email address (i.e. the @live.com email listed on your GH profile). But you'll have limited access to bug-reports/dashboard.
  • You to comment on the application pull request mentioning that you approve the integration.

I'll put together a draft PR shortly so that you can see what all this would look like.

@nathaniel-brough
Copy link
Contributor Author

Opened a draft PR with oss-fuzz here google/oss-fuzz#11189 once marked as not a draft this will act as both;

  • An initial integration test
  • Application to get rhai integrated with oss-fuzz.

@schungx
Copy link
Collaborator

schungx commented Nov 7, 2023

I have registered rhaiscript@gmail.com for things that need an email. Does this work?

@nathaniel-brough
Copy link
Contributor Author

Yeah that should work :)

@schungx
Copy link
Collaborator

schungx commented Nov 12, 2023

OK, the PR is merged.

@nathaniel-brough
Copy link
Contributor Author

Perfect I've marked the oss-fuzz application/integration as ready for review google/oss-fuzz#11189

@schungx
Copy link
Collaborator

schungx commented Nov 13, 2023

What should I do now?

@nathaniel-brough
Copy link
Contributor Author

Nothing for the moment, the oss-fuzz team will either reject/accept google/oss-fuzz#11189. Usually they respond within a couple of weeks but if not I'll ping them. Once that all goes through there won't be much more to do except improve the fuzzers and fix bugs.

After it's merged you'll start receiving bug reports on the gmail email address you created and you'll get access to the dashboard for rhai here https://oss-fuzz.com/login

Thanks for being so responsive with all this it definitely helps when things have some momentum.

@nathaniel-brough
Copy link
Contributor Author

Looks like everything is up and running. I got a couple of emails this morning with bug reports, so you should be able to login in to the the dashboard and take a look at those. Let me know if you have any further questions, or need help interpreting the bug reports:)

@schungx
Copy link
Collaborator

schungx commented Nov 19, 2023

Help will be great, as I don't even know how to login to the dashboard...

@nathaniel-brough
Copy link
Contributor Author

No worries, so head on over to https://oss-fuzz.com/login you'll be able to login with the rhaiscript@gmail.com email address that you created. You'll see something like this;
Screenshot_select-area_20231119101801

Clicking on the "open crashes" will give you a rundown of the bugs that the fuzzer has found. The crash statistics can also be useful to get general trends as to how the fuzzer is handling things.

@schungx
Copy link
Collaborator

schungx commented Nov 20, 2023

I clicked on the links from the emails and I successfully logged on to the dashboard.

I can now see the crash reports. They have great!

They can really catch edge cases.

@schungx
Copy link
Collaborator

schungx commented Nov 23, 2023

Fuzzing has been great to catch subtle bugs that sneaked through unit testing!

Thanks for setting this up!

@nathaniel-brough
Copy link
Contributor Author

No worries. I'm going to take a look at this one tomorrow.

Seems like it's either.

  1. Hitting a quadratic code path or
  2. It's a bug in my fuzz harness rather than rhai.

Let me know if you need help reproducing any of these bugs these locally. Its not always entirely obvious.

@schungx
Copy link
Collaborator

schungx commented Nov 23, 2023

Well, yes, this one is timed out, but I think probably due to complexity, as by default Rhai does not put a limit on operations count, so very complex expressions can run the parser for a long time...

Others, such as the stack overflows, are bugs, which I already fixed. Thanks to fuzzing!

@nathaniel-brough
Copy link
Contributor Author

Well, yes, this one is timed out, but I think probably due to complexity, as by default Rhai does not put a limit on operations count, so very complex expressions can run the parser for a long time...

Yeah I attempted to put a hard limit on the time here. So there is either a bug in my timeout code, or there a single operations is stuck taking more than 60s.

Others, such as the stack overflows, are bugs, which I already fixed. Thanks to fuzzing!

Are you certain that you fixed these bugs? Usually oss-fuzz will automatically detect if a bug is fixed and close each issue. This hasn't happened. So either there is a bug in oss-fuzz's bugfix detection system or the bugs haven't been fixed correctly. Just as a note, there have been a few issues with oss-fuzz's bugfix detection for rust in the last few months, so it's not entirely outlandish to think we might have run into one now. But if you are certain they are fixed but it's not being picked up we should file an issue with oss-fuzz.

@schungx
Copy link
Collaborator

schungx commented Nov 24, 2023

Are you certain that you fixed these bugs

Oh, I fixed the bugs in my own dev repo. I haven't pushed it to master yet. I'll do that.

@schungx
Copy link
Collaborator

schungx commented Nov 28, 2023

@silvergasp I have found that, even though I pushed new commits to the trunk, oss-fuzz continues to use an older commit for new tasks or progression tasks. Do you know how to force it to use the latest commit?

@nathaniel-brough
Copy link
Contributor Author

Hmm it should be using the latest commit. I'll look into it. It has definitely updated recently as I saw that those bugs that you fixed where marked as fixed automatically. But I'll look into it and make sure.

@nathaniel-brough
Copy link
Contributor Author

So I can confirm that it is indeed downloading the latest commit each night. If you go to the build-status page here and ctrl-F search for 'GIT_REV' you'll see the latest commit that was used.

$ git checkout 269302c7c8e6af028390c4e280012b5f2c2e409e
$ git log
commit 269302c7c8e6af028390c4e280012b5f2c2e409e (HEAD, upstream/main)
Merge: ee6261bf 33505173
Author: Stephen Chung <schungx@live.com>
Date:   Tue Nov 28 13:31:49 2023 +0800

    Merge pull request #787 from schungx/master
    
    Fix dynamic parameters bug.

@nathaniel-brough
Copy link
Contributor Author

Oh you'll need to click on the latest build tag as well e.g.

image

@schungx
Copy link
Collaborator

schungx commented Nov 29, 2023

Ah OK. I realize that I just have to wait until it builds the latest. Thanks!

@schungx
Copy link
Collaborator

schungx commented Nov 29, 2023

@silvergasp As far as I can tell, the time-out bugs were triggered based on a very very long and very very complicated script that causes the parser to run for a long time. Although Rhai controls run time, it does not control parsing time, because it is easy to reject long scripts before it gets to the parser.

I suggest we limit the input script length? Not sure how we can set this in the config...

@schungx
Copy link
Collaborator

schungx commented Nov 29, 2023

Also, I wonder whatever happens to the OptimizationLevel input... I never see it in test case data, and there is no indication which value was used in each run...

@nathaniel-brough
Copy link
Contributor Author

I suggest we limit the input script length? Not sure how we can set this in the config

Seems reasonable to me. We can probably just truncate the input string in the fuzzer itself. The fuzzer will learn that passing in larger scripts won't actually improve vicarage.

@nathaniel-brough
Copy link
Contributor Author

Also, I wonder whatever happens to the OptimizationLevel input... I never see it in test case data, and there is no indication which value was used in each run...

You should be able to see which value is used in the debug outputs. I'll post some instructions when I'm in front of a computer.

@nathaniel-brough
Copy link
Contributor Author

Ah apologies this slipped my, mind and now that I'm testing it all, I'm not sure why it's not printing the debug output when in repro mode... It's a little odd. It's easy enough to hack around e.g. just print the ctx object and then run the reproduction again.

println!("{ctx:?}");

You just probably wouldn't want to leave print statement in while fuzzing as it'll slow things down unnecesarily. So the steps would look something like this;

  1. Download the minimized test case from ossfuzz.com e.g.
    image

  2. Add the println statement as above;

  3. Run the fuzzer against the minimized test case e.g.
    cargo fuzz scripting <path_to_downloaded_testcase>

For whatever reason, when you find a bug for the first time while running cargo fuzz it'll print the debug representation for the crashing input. However when reproducing the bug as described above it doesn't... I'm going to open up an issue with rust-fuzz, as I feel like this is a bug. I don't see any reason to omit the debug print in reproduce mode.

@nathaniel-brough
Copy link
Contributor Author

Also as mentioned earlier, I've opened up a PR that will add fuzzing to the CI here #790

@nathaniel-brough
Copy link
Contributor Author

nathaniel-brough commented Dec 2, 2023

Regarding not being able to see the OptimizationLevel I've filed an issue with cargo-fuzz here rust-fuzz/cargo-fuzz#348. I don't feel like we should have to hack around this.

@schungx
Copy link
Collaborator

schungx commented Dec 4, 2023

@silvergasp There seems to be a new bug for oss-fuzz.

Data that contains invalid UTF8, when converted to String input, will crash at input preparation stage.

@nathaniel-brough
Copy link
Contributor Author

@silvergasp There seems to be a new bug for oss-fuzz.

Data that contains invalid UTF8, when converted to String input, will crash at input preparation stage.

Sorry I missed this one. I'm not sure that I understand the problem you've mentioned do you mind expanding on this a little?

Also re your query about printing the Ctx inputs I got a response from oss-fuzz here. Looks like it's pretty simple just cargo fuzz fmt fuzz_target_1 artifacts/fuzz_target_1/crash-adc83b19e793491b1c6ea0fd8b46cd9f32e592fc

@schungx
Copy link
Collaborator

schungx commented Jan 12, 2024

Sorry I missed this one. I'm not sure that I understand the problem you've mentioned do you mind expanding on this a little?

I believe my problem is not related to oss-fuzz and is already resolved. Sorry for the false alarm!

@schungx
Copy link
Collaborator

schungx commented Jan 17, 2024

@silvergasp crash https://oss-fuzz.com/testcase-detail/6378147608068096 is a time-out.

However, since it is mocking an object, is there any way to actually print out the object before running the fuzz?

What if we put in a println!("{:?}", ctx.all_types)?

Otherwise there won't be any way to tell what is causing the runaway behavior -- although I believe it is probably due to printing large structures over and over again and concatnating into strings...

@nathaniel-brough
Copy link
Contributor Author

I don't think the println! is neccesary. You should be able to print it using the fmt subcommand as described here.

cargo +nightly fuzz fmt fuzz_serde clusterfuzz-testcase-minimized-fuzz_serde-6378147608068096

@schungx
Copy link
Collaborator

schungx commented Jan 18, 2024

Well, it says like this:

Error: failed to run `cargo fuzz fmt` on input: clusterfuzz-testcase-fuzz_serde-4745120863813632

Caused by:
    Fuzz target 'fuzz_serde' exited with failure when attempting to debug formatting an interesting input that we discovered!

    Artifact: clusterfuzz-testcase-fuzz_serde-4745120863813632

    Command: "cargo" "run" "--manifest-path" "C:\\Git\\rhai\\fuzz\\Cargo.toml" "--target" "x86_64-pc-windows-msvc" "--release" "--bin" "fuzz_serde" "--" "-artifact_prefix=C:\\Git\\rhai\\fuzz\\artifacts\\fuzz_serde\\" "clusterfuzz-testcase-fuzz_serde-4745120863813632"

    Status: exit code: 1

    === stdout ===


    === stderr ===
    error: failed to run `rustc` to learn about target-specific information

    Caused by:
      process didn't exit successfully: `rustc - --crate-name ___ --print=file-names -Cpasses=sancov-module -Cllvm-args=-sanitizer-coverage-level=4 -Cllvm-args=-sanitizer-coverage-inline-8bit-counters -Cllvm-args=-sanitizer-coverage-pc-table -Cllvm-args=-sanitizer-coverage-trace-compares --cfg fuzzing -Clink-dead-code -Zsanitizer=address -Cdebug-assertions -Clink-arg=/include:main -C codegen-units=1 --target x86_64-pc-windows-msvc --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro --print=sysroot --print=split-debuginfo --print=crate-name --print=cfg` (exit code: 1)
      --- stdout
      ___.exe
      lib___.rlib
      ___.dll
      ___.dll
      ___.lib
      ___.dll
      C:\Users\Stephen\.rustup\toolchains\nightly-x86_64-pc-windows-msvc
      packed
      ___
      debug_assertions
      fuzzing
      overflow_checks
      panic="unwind"
      proc_macro
      relocation_model="pic"
      sanitize="address"
      target_abi=""
      target_arch="x86_64"
      target_endian="little"
      target_env="msvc"
      target_family="windows"
      target_feature="fxsr"
      target_feature="sse"
      target_feature="sse2"
      target_has_atomic
      target_has_atomic="16"
      target_has_atomic="32"
      target_has_atomic="64"
      target_has_atomic="8"
      target_has_atomic="ptr"
      target_has_atomic_equal_alignment="16"
      target_has_atomic_equal_alignment="32"
      target_has_atomic_equal_alignment="64"
      target_has_atomic_equal_alignment="8"
      target_has_atomic_equal_alignment="ptr"
      target_has_atomic_load_store
      target_has_atomic_load_store="16"
      target_has_atomic_load_store="32"
      target_has_atomic_load_store="64"
      target_has_atomic_load_store="8"
      target_has_atomic_load_store="ptr"
      target_os="windows"
      target_pointer_width="64"
      target_thread_local
      target_vendor="pc"
      windows

      --- stderr
      error: address sanitizer is not supported for this target

      error: aborting due to 1 previous error

@nathaniel-brough
Copy link
Contributor Author

Are you sure you ran it with +nightly address sanitizer isn't supported on stable (not that you should need it in this specific case).

@nathaniel-brough
Copy link
Contributor Author

Also this looks like it was run on windows, which doesn't support some sanitizer's maybe that's the problem. Do you have WSL installed?

@schungx
Copy link
Collaborator

schungx commented Jan 18, 2024

Also this looks like it was run on windows, which doesn't support some sanitizer's maybe that's the problem. Do you have WSL installed?

No...

Maybe I can use it on code space...

Or can you run it on your computer? I've attached the two test cases.

@schungx
Copy link
Collaborator

schungx commented Jan 18, 2024

testcases.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants