github / advisory-database

Security vulnerability database inclusive of CVEs and GitHub originated security advisories from the world of open source software.
Creative Commons Attribution 4.0 International
1.75k stars 336 forks source link

[GHSA-4f8r-qqr9-fq8j] Incorrect delegation lookups can make go-tuf download the wrong artifact #4893

Closed mamccorm closed 1 month ago

mamccorm commented 1 month ago

Updates

Comments There are two different packages of go-tuf:

The advisory data does not look correct - as the latest version of go-tuf (at the time of writing). is v0.7.0. However we're wrongly classifying that this is fixed in v2.0.1. There is no such version for this go-tuf package. The fix only exists in go-tuf/v2.

If we compare to here:

That data source correctly makes a distinction between the versions, and it does list there is no fixed version in go-tuf (i.e non-v2).

As further evidence, we can see here where they merged what looks to be the fix, into the v2.x release train:

But there is no such merge for v0.7.0, which had a release cut back in November 2023.

github commented 1 month ago

Hi there @rdimitrov! A community member has suggested an improvement to your security advisory. If approved, this change will affect the global advisory listed at github.com/advisories. It will not affect the version listed in your project repository.

This change will be reviewed by our Security Curation Team. If you have thoughts or feedback, please share them in a comment here! If this PR has already been closed, you can start a new community contribution for this advisory

westonsteimel commented 1 month ago

Why wouldn't we want to indicate that there is indeed an action that can be taken by upgrading to v2 though?

mamccorm commented 1 month ago

The main challenge here is that there are separate packages published with different names. For the old package, it results in a suggestion to upgrade to a newer image tag that doesn't exist, and also implies the non-v2 application has a fix available.

This is a much better representation and we should be consistent here:

shelbyc commented 1 month ago

👍 I think it's reasonable to set the vulnerable version range for github.com/theupdateframework/go-tuf to <= 0.7.0 and the vulnerable version range for github.com/theupdateframework/go-tuf/v2 to < 2.0.1. A typical person can see the two products and two VVRs next to each other and conclude that github.com/theupdateframework/go-tuf/v2 is a continuation of github.com/theupdateframework/go-tuf that includes patched versions on the 2.x branch.

Additionally, visitors to https://pkg.go.dev/github.com/theupdateframework/go-tuf are advised that The highest tagged major version is v2. and can be reasonably confident that these are different branches of the same software product.

advisory-database[bot] commented 1 month ago

Hi @mamccorm! Thank you so much for contributing to the GitHub Advisory Database. This database is free, open, and accessible to all, and it's people like you who make it great. Thanks for choosing to help others. We hope you send in more contributions in the future!

xnox commented 1 month ago

I think it's reasonable to set the vulnerable version range for github.com/theupdateframework/go-tuf to <= 0.7.0

is this correct? vulnerable code does not seem to appear in the 0.7.0 code base, also see this comment https://github.com/sigstore/sigstore/issues/1857#issuecomment-2407159536

@AdamKorcz and @rdimitrov can you please comment if 0.7.0 code base is affected or not?

rdimitrov commented 1 month ago

@mamccorm @xnox @shelbyc - Hey, we did a review of the legacy go-tuf code (v0.7.0) and it seems that it is not affected by the vulnerability referenced by this CVE.

How should we proceed so we can update the CVE to reflect this? Also should we update the GHSA?

cc: @haydentherapper @jku @kommendorkapten

xnox commented 1 month ago

@rdimitrov Hi, Please open a new PR, and use ranges of when introduced and when fixed.

For example in here https://github.com/github/advisory-database/blob/main/advisories/github-reviewed/2024/08/GHSA-25qx-vfw2-fw8r/GHSA-25qx-vfw2-fw8r.json the bug did affect two code branches, and was fixed in both.

Currently this GHSA is unbounded, as if the bug existed in the very first tag of the software. You can propose a PR, that updates modified timestamp; and then update introduced to "2.0.0" in the /v2 version stream. For the second range, you can change "last_affected": "0.7.0" to "fixed": "0.7.0" because at this point too many scanner and too many people are believing that 0.7.0 is vulnerable. Or maybe even delete the range.

But i'm not well versed in the format though.

shelbyc commented 1 month ago

@rdimitrov I see from commit https://github.com/theupdateframework/go-tuf/commit/edc30b474f5afd4cc603e17149704d5aa605151d that the vulnerable files were not introduced until version 2.0.0, so there is adequate evidence to show that 2.0.0 is the first vulnerable version.

I have changed the vulnerable version range in https://www.cve.org/CVERecord?id=CVE-2024-47534 to >= 2.0.0, < 2.0.1 because the CVE record uses the GitHub repository, not the package on pkg.go.dev.

In https://github.com/advisories/GHSA-4f8r-qqr9-fq8j, I have removed github.com/theupdateframework/go-tuf as an affected product entirely.

I've added https://github.com/theupdateframework/go-tuf/commit/edc30b474f5afd4cc603e17149704d5aa605151d as a reference link in both the CVE record and the advisory to show that only github.com/theupdateframework/go-tuf/v2 is vulnerable.

too many scanner and too many people are believing that 0.7.0 is vulnerable. Or maybe even delete the range.

@xnox I deleted the range because that provides the most accurate information. If anyone who thought 0.7.0 was vulnerable needs clarification, they can check https://github.com/theupdateframework/go-tuf/commit/edc30b474f5afd4cc603e17149704d5aa605151d.

rdimitrov commented 1 month ago

Thank you for helping sort this out, @shelbyc! 🙌

westonsteimel commented 1 month ago

@xnox @rdimitrov , I believe trivy use the go vuln db records directly for go matches rather than the GitHub advisory db records, so you may also need to suggest an improvement to https://pkg.go.dev/vuln/GO-2024-3166 to eliminate matches there