Cargo audit false positives for optional dependencies pulled in by disabled features.

[0xForerunner]

I'll give a minimal example here.
In my toml I have sqlx

sqlx = { version = "0.7", default-features = false, features = [
    "macros",
    "runtime-tokio-native-tls",
    "migrate",
    "postgres",
    "chrono",
] }

Checking my actual dependencies with cargo tree reveals:

cargo tree | grep sql                                                                                                                                                                                                                                                                                                                                                
│   ├── sqlx-core v0.7.3
│   │   ├── sqlformat v0.2.3
├── sqlx v0.7.3
│   ├── sqlx-core v0.7.3 (*)
│   ├── sqlx-macros v0.7.3 (proc-macro)
│   │   ├── sqlx-core v0.7.3
│   │   │   ├── sqlformat v0.2.3 (*)
│   │   ├── sqlx-macros-core v0.7.3
│   │   │   ├── sqlx-core v0.7.3 (*)
│   │   │   ├── sqlx-postgres v0.7.3
│   │   │   │   ├── sqlx-core v0.7.3 (*)
│   └── sqlx-postgres v0.7.3
│       ├── sqlx-core v0.7.3 (*)

And yet I still get the error

Crate:     rsa
Version:   0.9.6
Title:     Marvin Attack: potential key recovery through timing sidechannels
Date:      2023-11-22
ID:        RUSTSEC-2023-0071
URL:       https://rustsec.org/advisories/RUSTSEC-2023-0071
Severity:  5.9 (medium)
Solution:  No fixed upgrade is available!
Dependency tree:
rsa 0.9.6
└── sqlx-mysql 0.7.3
    ├── sqlx-macros-core 0.7.3
    │   └── sqlx-macros 0.7.3
    │       └── sqlx 0.7.3
    │           └── signup-sequencer 2.0.0
    └── sqlx 0.7.3

I understand this is due to cargo-audit simply scanning the lock file, but I imagine if cargo tree is smart enough to omit these deps then the same should be possible in cargo-audit. Let me know what you think!

[tarcieri]

Given cargo-audit's role as a Cargo.lock analyzer, there's little we can do without implementing a much more complicated analysis which can incorporate Cargo.toml as well, which has been discussed in the past.

IMO the real issue here is that the Cargo resolver is including these dependencies in Cargo.lock in the first place. I believe this issue might be relevant:

rust-lang/cargo#10801

[0xForerunner]

Yeah I can definitely see your point there. That's a pretty old issue though and this is a pretty important feature for a project like this. Would it be possible for some type of temporary solution when we run cargo tree in the background and filter results based off that? I know that feels a bit dirty, but perhaps warranted?

[tarcieri]

cargo-tree isn't exactly designed for programmatic consumption, and it's a heavy dependency.

To properly consume data from Cargo.toml files / Rust workspaces it would be better if we optionally linked with cargo the same way cargo-tree does. That could probably use its own issue (I'm kind of surprised we don't have a general tracking issue for it already, just a lot of discussion of potentially doing that).

[0xForerunner]

Gotcha. Okay well we've got this documented here for now. Let me know if you come up with any clever solutions to this, cheers!

[tarcieri]

Yeah, this is definitely a worthwhile issue to track, whether the solution happens upstream in Cargo or via new features added to cargo-audit.

[Shnatsel]

The way to go for filtering would be cargo metadata, but that runs into its own issues and limitations, as I've learned by working on cargo auditable.

[tarcieri]

The cargo issue I linked earlier suggests this issue occurs with cargo metadata as well

[Shnatsel]

It seems this issue is fixed in cargo deny: rust-lang/cargo#10801

@ewoolsey using cargo deny instead could be a reasonable workaround.

[Shnatsel]

Nevermind, I think it's only fixed for other lints, and security audits still run into this due to looking only at Cargo.lock

[stormshield-gt]

I think the issue is fixed inside the krate library that uses cargo deny under the hood. More specifically it was fixed in response to this EmbarkStudios/krates#41
Maybe using krate would be a viable option.

[tarcieri]

krate looks interesting!