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

x/vuln: incorrect fix version reporting GO-2023-1987, GO-2023-2041 #62276

Closed
notrobpike opened this issue Aug 25, 2023 · 6 comments
Closed

x/vuln: incorrect fix version reporting GO-2023-1987, GO-2023-2041 #62276

notrobpike opened this issue Aug 25, 2023 · 6 comments
Assignees
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. vulncheck or vulndb Issues for the x/vuln or x/vulndb repo

Comments

@notrobpike
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.20.6 darwin/amd64

Does this issue reproduce at the latest version of golang.org/x/vuln?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOOS="darwin"
GOARCH="amd64"

What did you do?

when checking a package using crypto/tls (in my case go-redis but anything will do), using go1.20.6, govulncheck reports vuln GO-2023-1987. which is valid. except that it reports it fixed in go1.21rc4.

What did you expect to see?

the vuln is fixed in an earlier version, go1.20.7. i expected govulncheck to report that version. i often see govulncheck report the earliest version where a vuln is fixed, so i expected that here as well.

What did you see instead?

Vulnerability #1: GO-2023-1987
    Large RSA keys can cause high CPU usage in crypto/tls
  More info: https://pkg.go.dev/vuln/GO-2023-1987
  Standard library
    Found in: crypto/tls@go1.20.6
    Fixed in: crypto/tls@go1.21rc4
@notrobpike notrobpike added the vulncheck or vulndb Issues for the x/vuln or x/vulndb repo label Aug 25, 2023
@gopherbot gopherbot modified the milestones: Unreleased, vuln/unplanned Aug 25, 2023
@cagedmantis
Copy link
Contributor

cc @golang/vulndb

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 25, 2023
@zpavlinovic
Copy link
Contributor

@tatianab

@notrobpike notrobpike changed the title x/vuln: incorrect fix version reporting GO-2023-1987 x/vuln: incorrect fix version reporting GO-2023-1987, GO-2023-2041 Sep 20, 2023
@notrobpike
Copy link
Author

notrobpike commented Sep 20, 2023

Also for the recent GO-2023-2041 and GO-2023-2043. Both report fixed in go1.21.1 but it's actually fixed earlier, in go1.20.8:

Vulnerability #2: GO-2023-2041
    Improper handling of HTML-like comments in script contexts in html/template
  More info: https://pkg.go.dev/vuln/GO-2023-2041
  Standard library
    Found in: html/template@go1.20.7
    Fixed in: html/template@go1.21.1

@zpavlinovic zpavlinovic self-assigned this Sep 20, 2023
@zpavlinovic
Copy link
Contributor

zpavlinovic commented Sep 21, 2023

govulncheck's current approach is to always report the latest fix available.

I think that often the earliest and the latest fix coincide, so that is why you sometimes see what you described (unless there is a bug in the latest fix computation).

In your case, is there a particular reason why you would prefer the earliest instead of the latest fix?

@notrobpike
Copy link
Author

notrobpike commented Sep 25, 2023

govulncheck's current approach is to always report the latest fix available.

huh. then the language should perhaps be changed. "found in / fixed in" implies first known occurrence and first known fix. At least to this reader.

In this case, it's because it is implying a need to update the minor version (1.21.x) instead of just a patch version (1.20.x). In go, even "minor" version updates can introduce breaking behavior that needs to be fully tested/qualified, eg in go1.21 the initialization order is better defined and may be different. (compatbility promise aside).

Besides resistance to upgrading minor version "just" to address a vuln, the latest version tends to have more churn.

The "fixed in" version for the original report, GO-2023-1987, is an rc version! govunlcheck is suggesting me to update my codebase to an rc version? I think that would be ok if that were the only fixed version, but this was fixed earlier, in a go1.20 release version.

I think that generally speaking, it would be much more useful to report the first fixed version.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/530679 mentions this issue: internal/scan: suggest earliest valid fixed version as the fix

@golang golang locked and limited conversation to collaborators Oct 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Projects
None yet
Development

No branches or pull requests

4 participants