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 fuzz integration #2022

Closed
wants to merge 1 commit into from
Closed

Initial fuzz integration #2022

wants to merge 1 commit into from

Conversation

manunio
Copy link

@manunio manunio commented Nov 21, 2023 •
edited
Loading

Description

This pr attempts to adds coverage guided fuzzing powered by jazzer.js with the aim that handlebars.js will eventually get integrated into oss-fuzz for continuous fuzzing. closes #1999

cc @jaylinski

Generally we like to see pull requests that

  • Please don't start pull requests for security issues. Instead, file a report at https://www.npmjs.com/advisories/report?package=handlebars
  • Maintain the existing code style
  • Are focused on a single change (i.e. avoid large refactoring or style adjustments in untouched code if not the primary goal of the pull request)
  • Have good commit messages
  • Have tests
  • Have the typings (types/index.d.ts) updated on every API change. If you need help, updating those, please mention that in the PR description.
  • Don't significantly decrease the current code coverage (see coverage/lcov-report/index.html)
  • Currently, the 4.x-branch contains the latest version. Please target that branch in the PR.

@manunio manunio marked this pull request as draft November 21, 2023 15:59
@manunio manunio marked this pull request as ready for review November 21, 2023 16:58
@jaylinski jaylinski self-assigned this Nov 21, 2023
@manunio
Copy link
Author

manunio commented Nov 29, 2023

Hi @jaylinski would be interested in fuzzing Handlebars.js at oss-fuzz ?
It's a free service by google for continuously fuzzing important open source projects.
As an initial step i have created this pr and a pr at oss-fuzz: google/oss-fuzz#11281 for initial integration.

If you are interested, oss-fuzz requires a email(s) address from your end.
This helps ensure that project members get bug reports effectively, and the email must be a
google account why-do-you-require-a-google-account-for-authentication.
Note the email(s) affiliated with the project will be public in the oss-fuzz repo, as they will be part of a configuration file.

jaylinski
jaylinski reviewed Nov 29, 2023
View reviewed changes
Copy link
Member

@jaylinski jaylinski left a comment

Choose a reason for hiding this comment

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

Thanks, looks good!

Before merging, I'm thinking about running your fuzz-tests in a GitHub action for about 2 minutes. This ensures that the script won't get broken and new (shallow) bugs will be detected before merging to master.

What do think about it?

fuzz/README.md Show resolved Hide resolved
@manunio
Copy link
Author

manunio commented Nov 29, 2023 •
edited
Loading

Thanks, looks good!

Before merging, I'm thinking about running your fuzz-tests in a GitHub action for about 2 minutes. This ensures that the script won't get broken and new (shallow) bugs will be detected before merging to master.

What do think about it?

If your aim for this is to get some initial finding, I have already tested it by running for sometime at my local machine , and have not found anything as of now. and to check whether this script is broken or not i have tested it against oss-fuzz CI at google/oss-fuzz#11281, you can see the all checks have passed there.

On side note OSS-Fuzz offers CIFuzz, a GitHub action/CI job that runs your fuzz targets on pull requests(for projects added to oss-fuzz).

Edit:
If you don't want to join oss-fuzz, we can try ClusterFuzzLite which is a continuous fuzzing solution that runs as part of Continuous Integration (CI) workflows to find vulnerabilities faster than ever before. With just a few lines of code, GitHub users can integrate ClusterFuzzLite into their workflow and fuzz pull requests to catch bugs before they are committed. ClusterFuzzLite is based on ClusterFuzz which powers oss-fuzz.

@manunio
Copy link
Author

manunio commented Dec 3, 2023

@jaylinski friendly ping :)

@jaylinski
Copy link
Member

I'd still like to run the fuzzing on GitHub CI just to verify/document the script.

Can you add a pipeline script? (If not, I also can add it.)

@manunio
Copy link
Author

manunio commented Dec 6, 2023

I'd still like to run the fuzzing on GitHub CI just to verify/document the script.

Can you add a pipeline script? (If not, I also can add it.)

Sounds good, I'll add it :)

manunio
manunio commented Dec 6, 2023
View reviewed changes
fuzz/test.sh Show resolved Hide resolved
@manunio
fuzz: initial integration
f1fffb4 
 - Adds initial fuzz_target for Handlebars.compile()
 - Adds Jazzer.js as dev-dependency
 - Adds fuzzing on Github CI
@manunio
Copy link
Author

manunio commented Dec 11, 2023

Hi @jaylinski friendly ping :)

jaylinski
jaylinski reviewed Dec 12, 2023
View reviewed changes
Copy link
Member

@jaylinski jaylinski left a comment

Choose a reason for hiding this comment

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

Thanks for your work so far!

Another thing I just thought about: I'm curious if this fuzzing setup would actually find previously fixed vulns (https://snyk.io/advisor/npm-package/handlebars).
In case of a code-injection vulnerability, I guess the fuzzer would have to generate a throw-statement in order to trigger the try-catch-block?

.github/workflows/ci.yml Show resolved Hide resolved
@manunio
Copy link
Author

manunio commented Dec 12, 2023

Thanks for your work so far!

Another thing I just thought about: I'm curious if this fuzzing setup would actually find previously fixed vulns (https://snyk.io/advisor/npm-package/handlebars).
In case of a code-injection vulnerability, I guess the fuzzer would have to generate a throw-statement in order to trigger the try-catch-block?

Fuzzing can be effective in finding vulnerabilities, including those that were previously fixed. However, it depends on the specific techniques used in the fuzzer and the nature of the vulnerabilities.

For code injection vulnerabilities, the fuzzer might try to generate inputs that manipulate the program's control flow. In the case of a try-catch block, attempting to trigger it with a crafted throw statement could be one approach to uncovering code injection issues.

Keep in mind that the success of finding such vulnerabilities also relies on the fuzzer's ability to explore different code paths and exercise edge cases.

@manunio
Copy link
Author

manunio commented Dec 22, 2023

All checks are green :)

@manunio
Copy link
Author

manunio commented Jan 7, 2024

Hi @jaylinski Can we please get this merged?

@manunio
Copy link
Author

manunio commented Jan 28, 2024

ping :)

@manunio manunio closed this May 23, 2024
@manunio manunio deleted the feat-jazzer-integration branch May 23, 2024 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaylinski jaylinski Awaiting requested review from jaylinski

Assignees

@jaylinski jaylinski

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add automated fuzzing
2 participants
@manunio @jaylinski