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

Pre-releases are not distinguished from releases #455

Closed
pinkforest opened this issue Sep 12, 2022 · 2 comments · Fixed by #464
Closed

Pre-releases are not distinguished from releases #455

pinkforest opened this issue Sep 12, 2022 · 2 comments · Fixed by #464
Labels
bug Something isn't working

Comments

@pinkforest
Copy link

pinkforest commented Sep 12, 2022

Describe the bug

axum-core here: rustsec/advisory-db#1417 (comment) has the below pattern:

    0.2.7 (and anything below) is vulnerable
    0.2.8 (and anything until 0.3.0.rc.1) is not vulnerable
    0.3.0-rc.1 is vulnerable
    0.3.0-rc.2 (and anything above) is not vulnerable
    0.3.0 eventually as real release will not be expected to be vulnerable

It's a problematic pattern as we cannot currently say for various reasons including resulting implicit arithmetric:

patched = [">= 0.2.8, < 0.3.0", ">= 0.3.0"]

Mostly above fails due to overlapping versions and we went through several iterations -

Until we came up with something like this:

patched = ["= 0.2.8", "= 0.3.0", ">= 0.3.1"]
unaffected = ["= 0.3.0-rc.2"]

Tested with cargo audit: rustsec/advisory-db#1417 (comment) - works as expected
And tested with cargo deny: rustsec/advisory-db#1417 (comment) - doesn't recognise 0.3.0-rc.1 as affected

Some pre-releases / release candidates can be vulnerable and are different to official releases.

e.g. in case of axum-core there is vulnerability that affects 0.3.0-rc.1 where as 0.3.0=rc.2 and future non-pre-release 0.3.0 will be patched but cargo deny doesn't distinguish between pre-release and release.

cargo deny does not flag 0.3.0-rc.1 as expected but at least it doesn't cause other side effects and does a warning of some sort:

2022-09-12 12:18:20 [WARN] unable to find a config path, falling back to default config
warning[A009]: advisory for a crate with a pre-release was skipped as it matched a patch
  ┌─ /home/foobar/gg/play/zz/tesssts/Cargo.lock:2:1
  │
2 │ axum-core 0.3.0-rc.1 registry+https://github.com/rust-lang/crates.io-index
  │ -------------------------------------------------------------------------- pre-release crate
  │
  = ID: RUSTSEC-2022-4000
  = Advisory: https://rustsec.org/advisories/RUSTSEC-2022-4000
  = Satisfied version requirement: =0.3.0
  = axum-core v0.3.0-rc.1
    └── tesssts v0.1.0

advisories ok

cargo audit flags it correctly:

$ grep axum-core Cargo.toml; cargo audit -n

axum-core = "=0.3.0-rc.1"
      Loaded 457 security advisories (from /home/foobar/.cargo/advisory-db)
    Scanning Cargo.lock for vulnerabilities (18 crate dependencies)
Crate:         axum-core
Version:       0.3.0-rc.1
Title:         No default limit put on request bodies
Date:          2022-08-31
ID:            RUSTSEC-2022-4000
URL:           https://rustsec.org/advisories/RUSTSEC-2022-4000
Solution:      Upgrade to =0.2.8 OR =0.3.0 OR >=0.3.1
Dependency tree: 
axum-core 0.3.0-rc.1
└── tesssts 0.1.0

error: 1 vulnerability found!

To Reproduce

Dummy crate that relies with release-candidate:

[dependencies]
axum-core = "=0.3.0-rc.1"

Then mock advisory with how we would mark it in advisory db:

patched = ["= 0.2.8", "= 0.3.0", ">= 0.3.1"]
unaffected = ["= 0.3.0-rc.2"]

Expected behavior

cargo deny should flag advisory on 0.3.0-rc.1 when patched is pinned = 0.3.0 and 0.3.0-rc.1 has not been marked as unaffected

Additional context

I don't think we've added advisories that need to match pre-releases but worth an ask if cargo deny could support this I guess ?

Discussing here in depth what to do with pre-releases in general: rustsec/rustsec#690

@pinkforest pinkforest added the bug Something isn't working label Sep 12, 2022
@Jake-Shadle
Copy link
Member

I'd accept a patch for this because I know the handling is incorrect for pre-releases, but they are also annoying and relatively rare so it's IMO low priority.

@pinkforest
Copy link
Author

pinkforest commented Sep 12, 2022

Yeah cool just figuring what our canonical is and happy to send a patch after,

I've asked others around preferences but I think: rustsec/rustsec#690 (comment) may end up as our intended canonical.

e.g. in axum-core we would end up with 0.3.0 being patched with simply this:

patched = [">= 0.2.8, < 0.3.0-rc.1", ">= 0.3.0-rc.2"]

As our canonical would be lexicographic and pre-releases would be "lesser than" release:

0.3.0-rc.1
0.3.0.rc.2
0.3.0  <-- release after pre-releases is "greater" than pre-releases so the above pattern works to catch it in "patched"

Problem would be in any tooling where SemVer 0.3.0 is seen as "before" or "lesser than" pre-releases e.g. in a case where 0.3.0 would still end up as vulnerable if patched is marked as >= 0.3.0-rc.2 so I'm just going around to inquire some preferences / expectations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants