-
Notifications
You must be signed in to change notification settings - Fork 361
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
Missing documentation #25
Comments
Hi there! Some great questions, thanks for opening this issue. For context this is a project I spiked out last year and have probably not devoted enough time to maintaining/improving after it hit something close to an MVP, but I'd love to work through these issues now and better document the project. RustSec questions
You are correct that opening a PR is the correct way to file an advisory. We should probably add a The original impetus behind this project was a
I would categorize both of these as a potential DoS if they can be induced by an attacker. I just merged a panic-related issue in #24.
As in something that miscalculates a buffer size and returns e.g. a
File away here. So long as the issues are legitimate I will merge them into the advisory database, with our without feedback from the crate maintainers (preferably with, but I'd personally rather err on the side of comprehensiveness)
Haha fun! I'd suggest opening both an issue on the upstream project and advisories here, and cross-referencing them in the issue description/comments.
I haven't exactly done a great job popularizing RustSec, especially since its release. It's linked from the Cargo Subcommands documentation but not really anywhere else to my knowledge. Adding references to it in the Cargo documentation is a great idea.
You might take a look at the cargo advisory rust-internals thread (in addition to rust-lang/rfcs#1752) for some history of how RustSec came about. The verdict at the time was directly integrating a feature like this into Cargo was premature. However, perhaps if you can fill up the database with vulnerabilities, it will be a good time to discuss upstreaming it into Cargo proper again.
That is also a great idea, particularly for downstream dependencies. Generic Rust Questions
It seems like an attribute in
There are mixed opinions on yanking but I think yanking versions with severe vulnerabilities (e.g RCE) would probably be for the best. That's just my opinion though. That said RustSec is designed to alert people to vulnerabilities without requiring crate publishers yank every last vulnerable crate. |
Thanks for the swift and comprehensive reply!
cargo-fuzz trophy case is a good starting point for filling up the DB. I think it's a chicken-and-egg problem: people are not aware of a way to announce security fixes, so they do not announce them and it doesn't give them much visibility. It should be fairly easy to test this hypothesis by scrubbing bug trackers for popular crates for keywords like "panic" or "overflow"; I'm sure they're there. For reference, here are the somewhat security-related bugs I've found so far: Also, I'd appreciate documentation on specifying version ranges for vulnerabilities, e.g. how to express that 1.2.0 and 2.0.0 are vulnerable, but 1.2.1 and 2.0.1 are not. |
FWIW, I wouldn't bother including panics as security issues. (a) I think it'll bloat the DB, reducing it's utility, (b) most other folks don't count them this way; I'm not aware of any browser or OS that include null-derefs in advisories for example, OSS-Fuzz marks null-deref as regular bugs, not security. |
Regarding a) I'm not particularly concerned about bloating the database as we presently have a total of 6 published advisories, so if anything I'd like to encourage more contributions. As for b) from what I've observed crashes/DoS advisories are published quite frequently for at least Windows and OS X/iOS. I think #24 in particular warrants inclusion. |
The problem is exactly in this difference of opinion: some people will assume they are important and report them, while others will not, resulting in wildly incomplete coverage of panics. Also, upon seeing advisories about panics, some people will consider them noise (or at least something that does not warrant upgrading to an API with a breaking change) and ignore actually critical advisories as well. So I'd suggest only including panics if the crate specifically guarantees that it will not panic. |
That's a good metric. And I'd agree we should have some clearer guidelines about what sort of vulnerabilities should be reported to the database. Seems like good content for a |
During my recent work on fuzzing popular Rust crates I've noticed that there are basically two kinds of DoS issues: panics that unwind the current thread and resource exhaustion issues (memory leaks, unbounded allocations, stack overflow, etc) that crash the entire process. I think it would make sense to omit panics unless the crate explicitly guarantees that it won't panic, because interested parties can easily guard against them by isolating unreliable code in a separate thread. I would include DoS issues that crash the entire process because you cannot reasonably guard against them in a DoS-critical application. |
@Shnatsel those criteria seem reasonable to me |
This is long overdue! (see #25) It provides basic instructions for filing advisories against the database, and also some guidelines for what types of vulnerabilities qualify.
This is long overdue! (see #25) It provides basic instructions for filing advisories against the database, and also some guidelines for what types of vulnerabilities qualify.
Here is a PR for an initial CONTRIBUTING.md (Edit: Now merged, but please feel free to leave feedback or send PRs against it) |
I think we should be good on this now. Please reopen if you disagree. |
I was unable to comment earlier, but this is great, thanks! I might or might not propose some clarifications to the criteria for whether or not a vulnerability qualifies. Also, I really dig the Big Red Button in the readme. |
By looking at this repository I do not see any answers to the following questions:
unsafe
by API consumer? I've already found all of these.The reason I'm asking is that I've spent the last several days fuzzing various popular library crates, so now I'm sitting on a pile of bugs, wondering what to do with them aside of fixing them and opening PRs in absence of action from maintainers.
Also, somewhat out of scope of this issue:
cargo audit
on everything twice a day, even if I'd known about it. Yet the readme for it says it doesn't even ship with cargo by default?The text was updated successfully, but these errors were encountered: