RUSTSEC-2020-0011 is not a security vulnerability.

[hdevalence]

PR #268 added a security advisory related to plutonium, a crate that hides unsafe usage.

However, the security advisory does not report a security issue with the crate, or any defect with its intended functionality, but makes a value judgement about whether the crate's intended behavior is good.

Security advisories are not for "crates we don't like", they're for conveying information about defects in intended behavior.

[tarcieri]

It’s definitely an interesting and unusual case.

It might make sense to create a new class of informational advisories for cases like this, as those are the existing mechanism for publishing advisories which aren’t specifically an advisory of a security vulnerability (presently they are used exclusively for tracking unmaintained crates). Informational advisories are surfaced as warnings instead of vulnerabilities.

[Shnatsel]

I see it was filed as a regular security advisory rather than a special "informational" advisory type we use for e.g. unmaintained crates (example). @najamelan how do you feel about downgrading the advisory to a warning? This will still allow using it in cargo-deny, which I believe was the original motivation for the advisory.

[hdevalence]

The crate isn't unmaintained, though. By all appearances it implements its intended functionality perfectly, the only problem is that some people don't like that functionality.

[myrrlyn]

Co-signed. As written, the crate is a modification to the visual representation of Rust source code; it does not represent or use a vulnerability in either the compiler’s intake parser or output generator. The only change it makes is in the reader-facing representation of a narrow part of syntax.

plutonium represents a point of discussion about the philosophical approach that we as authors and viewers of Rust syntax take with regard to a class of operation. It does not represent any flaw in either the production or execution of a program, and as such, does not qualify as a vulnerability.

[alex]

this is code that let's you write memory unsafe code without the unsafe keyword. That's unsoundness. When it happens by accident it's a security issue, when it happens on purpose, it still is.

[myrrlyn]

No, it is not unsoundness. Unsoundness is specifically a term for the emission of a class of incorrect executable instructions, or a class of semantic input that causes the compiler to construct an incorrect view of the abstract program. It does not apply to the dialect of syntax that viewers observe.

[tarcieri]

The crate isn't unmaintained, though. By all appearances it implements its intended functionality perfectly, the only problem is that some people don't like that functionality.

Informational advisories are modeled as an enum:

https://github.com/RustSec/rustsec-crate/blob/ee8b2e5/src/advisory/informational.rs#L10-L21

pub enum Informational {
    /// Security notices for a crate which are published on <https://rustsec.org>
    /// but don't represent a vulnerability in a crate itself.
    Notice,

    /// Crate is unmaintained / abandoned
    Unmaintained,

    /// Other types of informational advisories: left open-ended to add
    /// more of them in the future.
    Other(String),
}

I think we could probably use the catch-all "notice" category for this purpose. This surfaces as a warning in cargo audit.

[andrew-d]

FWIW, my vote is that some sort of notification is correct here. To draw a comparison: if a crate bundled and made use of a shared object that contained a vulnerability, I would want to know about that, even if all the Rust code in the crate was safe/sound since the expected behavior ("this crate is safe and behaves as expected") is violated. Similarly, plutonium subverts the expectations and security guarantees of a developer that transitively depends on it, in a way that could result in unsafety or unsoundness, and that meets the bar for a reportable vulnerability to me.

[najamelan]

I'm fine with this being a warning. I just feel this should never slip unnoticed into a dependency graph, and the crate was deliberately designed to exploit a limitation of cargo-geiger so people wouldn't notice they have unsafe in certain dependencies.

It is further problematic as grepping for the string unsafe is a common review technique disabled by this crate, which warrants a warning in my opinion.

I can only conclude that the intend and purpose of this crate is to make reviewing for memory safety harder, which is not something I think should be on crates. If people want to write unsafe code without using the keyword at home, I couldn't care less, but this invites anyone not wanting hassle for using unsafe to just add it as a dep in Cargo.toml and I'd rather we don't make that easier and less transparent.

[myrrlyn]

If we're going to debate the virtue of a source file, then I'd like to respectfully rejoin that, for example, I am the author of a crate with a ridiculously high unsafe metric despite actually only having a select few parts of the codebase that are actually unsafe. The #[safe] annotation allows authors and readers alike to distinguish between "this performs an action that, while safe, is not registered as such with the compiler", and "this is unsafe and warrants significant attention".

This crate provides an opportunity to decrease the background-noise present and increase the signaling capacity of the unsafe keyword, so that rg unsafe becomes actually useful.

[najamelan]

@myrrlyn I'm sorry we disagree, but as a reviewer "these uses of unsafe don't merit review" is one of the assumptions you definitely want to double check, so you want to find all occurrences and not only the ones the author thought were maybe dangerous.

[Manishearth]

I don't think "this circumvents cargo-geiger" is enough justification. cargo-geiger is a separate tool, and one with a flawed metric that this crate neatly illustrates. If cargo-geiger users care about this crate being caught, cargo-geiger should handle that, not the rust security advisory database.

I don't quite agree with @myrrlyn -- I don't think one should use this crate in production code -- however, his underlying point about degrees of unsafe is one I strongly agree with. cargo-geiger does nothing to help there. Perhaps that's not the point of the tool, but it's just one of the problems with treating the count of unsafe as a security metric. At best it gives a rough filtering to areas of code you might want to look at closer.

And this advisory database is about security.

[hdevalence]

@najamelan

I can only conclude that the intend and purpose of this crate is to make reviewing for memory safety harder, which is not something I think should be on crates.

The thing is, security advisories are not about what you think should be on crates.io.

There is no security issue here. There should not be a security advisory, informational or otherwise. It devalues the meaning of this database.

Let's be clear: we're talking about a crate with 0 reverse dependencies and (as of this writing) 102 downloads by anyone, ever. The only reason there's an "advisory" is because someone saw it mentioned on Reddit and decided they didn't like it.

[sunjay]

If cargo-geiger users care about this crate being caught, cargo-geiger should handle that, not the rust security advisory database.

+1 on this and the rest of Manish's comment as well. I think this particular sentence should be the main focus of this issue. The security advisory should be removed and a check for this should be added to cargo-geiger if that is deemed necessary.