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

Integrate initial OSS-Fuzz support #5850

Open
2 tasks done
initializedd opened this issue Dec 21, 2024 · 8 comments · May be fixed by #5851
Open
2 tasks done

Integrate initial OSS-Fuzz support #5850

initializedd opened this issue Dec 21, 2024 · 8 comments · May be fixed by #5851
Labels
C-enhancement Category: Raise on the bar on expectations

Comments

@initializedd
Copy link

initializedd commented Dec 21, 2024

Please complete the following tasks

Clap Version

master

Describe your use case

Initial support for fuzzing clap to discover and fix bugs.

If my fuzzer is merged, I will open a pull request in the OSS-Fuzz repository to run the fuzzers for this library on Google's infrastructure. Maintainers of clap will be notified if any bugs are discovered.

Please see the OSS-Fuzz documentation and Bug disclosure guidelines before merging.

Thanks!

Describe the solution you'd like

PR #5851

Alternatives, if applicable

No response

Additional Context

No response

@initializedd initializedd added the C-enhancement Category: Raise on the bar on expectations label Dec 21, 2024
initializedd added a commit to initializedd/clap that referenced this issue Dec 21, 2024
@initializedd initializedd linked a pull request Dec 21, 2024 that will close this issue
initializedd added a commit to initializedd/clap that referenced this issue Dec 21, 2024
initializedd added a commit to initializedd/clap that referenced this issue Dec 21, 2024
initializedd added a commit to initializedd/clap that referenced this issue Dec 21, 2024
@epage
Copy link
Member

epage commented Dec 24, 2024

I've wondered about fuzzing but whats not too clear to me is what would provide the best value, or in other words, what are the high risk areas. Like in #5851, its parsing strings (not OsStrings) against a limited, static CLI definition. I feel like this is unlikely to uncover anything of interest.

@initializedd
Copy link
Author

I've wondered about fuzzing but whats not too clear to me is what would provide the best value, or in other words, what are the high risk areas. Like in #5851, its parsing strings (not OsStrings) against a limited, static CLI definition. I feel like this is unlikely to uncover anything of interest.

Thanks for your response.

I agree that the fuzzer in #5851 is unlikely to uncover anything of interest, but I’d like to emphasize that that this is more about integrating OSS-Fuzz support than the specific fuzzers themselves at this stage. We need to ensure that we have the necessary approvals from both parties before diving deep into the fuzzers. The aim is to eventually reach 100% fuzzing coverage through an iterative process of constant refinement.

I've updated #5851 to parse OsStrings instead.

I am open to any further suggestions you may have.

@epage
Copy link
Member

epage commented Jan 2, 2025

An OsString from a String doesn't make much of a difference.

For me, the big question is "will this be worth it" which is dependent on what a proposed fuzzer looks like.

@popematt
Copy link

popematt commented Jan 7, 2025

@initializedd, I'm sorry, I over-reacted based on my own bad experience with some unknown 3rd party actor. I realize now that it sounds like I am accusing you of malicious behavior. I know nothing about you, and so I cannot fairly comment on your intentions. In retrospect, I don't think my original comment has any value, so I will delete it.

@initializedd
Copy link
Author

initializedd commented Jan 7, 2025

@initializedd, I'm sorry, I over-reacted based on my own bad experience with some unknown 3rd party actor. I realize now that it sounds like I am accusing you of malicious behavior. I know nothing about you, and so I cannot fairly comment on your intentions. In retrospect, I don't think my original comment has any value, so I will delete it.

Thanks for understanding.

@epage Are there any specific examples that you would suggest drawing inspiration from to improve the proposed fuzzer?

@epage
Copy link
Member

epage commented Jan 7, 2025

I don't know of an example that would be relevant for fuzzing. What I'm asking of you is to propose fuzzing that would provide enough value to be worth it.

@initializedd
Copy link
Author

@epage Apologies for my absence.

I am still keen to integrate fuzzing support for your library.

I've taken a further look at the codebase, and it seems to me that we will get the most value out of this by fuzzing the internal functions. Usages of unsafe (although few in the codebase) should probably be prioritized for extra assurance.

Some ideas of internal functions that we could start with:

Absolutely happy to implement support for all of those (providing that you agree), and any more that you think would be good.

@epage
Copy link
Member

epage commented Mar 17, 2025

Note that the goal for any of those ext.rs functions is for them to be moved into std and for us to use that, see rust-lang/libs-team#311

clap_lexs public API is a possibility, depending on what the fuzzing would like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Raise on the bar on expectations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants