Dependabot security alerts for Rust recommend updating a dependency that is only indirect, which is not the right fix

[daira]

Is there an existing issue for this?

• I have searched the existing issues

Feature description

If a project using Dependabot has (only) an indirect dependency on a vulnerable Rust crate, the security alert does not distinguish that case and will recommend a fix that would only be applicable for a direct dependency.

Example vulnerability: mio recently had a (Windows-specific) security vulnerability, tokio-rs/mio#1760. mio is an optional dependency of tokio, which is widely depended on, but most projects that depend on tokio do not depend directly (if at all) on mio.

Example downstream project: Dependabot is configured (dependabot.yml) for zcash/librustzcash, which is a workspace containing several library crates with a shared Cargo.lock. Some of its component crates have an indirect dependency or dev-dependency on tokio, but none of them have a direct dependency on mio:

[redacted]/librustzcash$ cargo tree --all-features --target=all -i mio
mio v0.8.9
└── tokio v1.35.1
    ├── h2 v0.3.21
    │   ├── hyper v0.14.27
    │   │   ├── axum v0.6.20
    │   │   │   └── tonic v0.10.2
    │   │   │       └── zcash_client_backend v0.11.0 ([redacted]/librustzcash/zcash_client_backend)
    │   │   │           └── zcash_client_sqlite v0.9.0 ([redacted]/librustzcash/zcash_client_sqlite)
    │   │   │           [dev-dependencies]
    │   │   │           └── zcash_client_sqlite v0.9.0 ([redacted]/librustzcash/zcash_client_sqlite)
    │   │   ├── hyper-timeout v0.4.1
    │   │   │   └── tonic v0.10.2 (*)
    │   │   └── tonic v0.10.2 (*)
    │   └── tonic v0.10.2 (*)
    ├── hyper v0.14.27 (*)
    ├── hyper-timeout v0.4.1 (*)
    ├── tokio-io-timeout v1.2.0
    │   └── hyper-timeout v0.4.1 (*)
    ├── tokio-stream v0.1.14
    │   └── tonic v0.10.2 (*)
    ├── tokio-util v0.7.10
    │   ├── h2 v0.3.21 (*)
    │   └── tower v0.4.13
    │       ├── axum v0.6.20 (*)
    │       └── tonic v0.10.2 (*)
    ├── tonic v0.10.2 (*)
    └── tower v0.4.13 (*)

librustzcash got a security alert at https://github.com/zcash/librustzcash/security/dependabot/8 that looked like this:

At the time of the alert, tokio depended (optionally) on mio 0.8.9. In this case, because mio 0.8.9 → 0.8.11 is a non-breaking update according to Semver:

• A cargo update (or more narrowly, cargo update -p mio) would be sufficient to update Cargo.lock in librustzcash.
• If tokio were to release a version that updated its dependency on mio (see Windows Named pipes invalid memory access tokio-rs/tokio#6369 (comment)), the librustzcash project might prefer to update its dependency on tokio (more precisely, the dependency chain tonic → axum → hyper → h2 → tokio) to that version — even though it isn't strictly required because librustzcash only contains library crates and the fix to mio is a non-breaking update. But such a version doesn't necessarily exist (at time-of-writing, it didn't in this example) when the security alert occurs and/or when a PR to fix it is created.
• Adding a new dependency on mio would be incorrect and useless.

If, on the other hand, the fixed version of the vulnerable crate were a breaking update according to Semver, then adding a dependency on the new version would not fix the potential vulnerability, because the crate dependency would be duplicated. And so for indirect dependencies, the advised action is either useless (if it is not also a direct dependency) or insufficient.

The main advised action should be to run cargo update -p mio, and then check using cargo tree --all-features --target=all -i mio that the project now only pins non-vulnerable versions. (In this case the mio dependency isn't duplicated, but if it were then you would need to run cargo tree ... for each version.)

The PR created by the big green "Create Dependabot security update" button is correct and indeed does the equivalent of cargo update -p mio: zcash/librustzcash#1225. So it's just the security alert text that could be misleading.

Note that there are many cases here. The project using dependabot:

• may or may not have a checked-in Cargo.lock;
• may or may not be a library crate that other projects could depend on (note that the advice on whether to check in Cargo.lock for libraries has changed);
• could depend on the vulnerable crate both directly and indirectly;
• could depend on the vulnerable crate via a chain, or multiple chains, of intermediate dependencies;
• could depend on, or pin, multiple versions of the vulnerable crate;
• could have different dependencies for different features and/or targets;
• could be a workspace with multiple Cargo.tomls and any of the above applying to each component crate;
• may have any configured versioning strategy.