False positives when dependency versioning isn't go semver

First of all, thanks for all your work maintaining nancy. We're using it in a CI pipeline on this repo, and have recently hit a snag which I think might affect others eventually.

What are you trying to do?

We require github.com/prometheus/prometheus which for some reason does not adhere to go semver. This also affects others who import it.

Our solution is to require the version we want and then replace it with the SHA version, for example:

require github.com/prometheus/prometheus v2.23.0+incompatible

replace (
    github.com/prometheus/prometheus => github.com/prometheus/prometheus v0.0.0-20201126101154-26d89b4b0776
)

In this case, v0.0.0-20201126101154-26d89b4b0776 is the SHA version for the tag v2.23.0.

However, using this method, nancy still reports a vulnerability which was present in versions < 2.7.1. It would be great if nancy was able to determine the "actual" version.

I made a minimal working example at https://github.com/stone-z/nancy-test.

What feature or behavior is this required for?

This false positive means we simply have to exclude a CVE, but I suspect that this situation might arise with other repositories and result in nancy yielding incorrect results.

How could we solve this issue? (Not knowing is okay!)

Admittedly, since prometheus breaks the rules I'm not sure what the best solution is here, or if it makes sense to implement. However, as not every repository uses proper go semver for versioning, it would be great if there was some awareness of tag versions in these circumstances in order to reduce the false positives.

Anything else?

If it helps, here is the full command output:

$ go list -json -m all | ./nancy-linux.amd64v1.0.1 sleuth --quiet

Vulnerable Packages

[1/1]   pkg:golang/github.com/prometheus/prometheus@0.0.0-20201126101154-26d89b4b0776
1 known vulnerabilities affecting installed version 
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ [CVE-2019-3826]  Improper Neutralization of Input During Web Page Generation ("Cross-site Scripting")                                                                                                                                   ┃
┣━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Description        ┃ A stored, DOM based, cross-site scripting (XSS) flaw was found in                                                                                                                                                  ┃
┃                    ┃ Prometheus before version 2.7.1. An attacker could exploit this by                                                                                                                                                 ┃
┃                    ┃ convincing an authenticated user to visit a crafted URL on a Prometheus                                                                                                                                            ┃
┃                    ┃ server, allowing for the execution and persistent storage of arbitrary                                                                                                                                             ┃
┃                    ┃ scripts.                                                                                                                                                                                                           ┃
┣━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ OSS Index ID       ┃                                                                                                                                                                                                                    ┃
┣━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ CVSS Score         ┃ 5.4/10 (Medium)                                                                                                                                                                                                    ┃
┣━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ CVSS Vector        ┃ CVSS:3.0/AV:N/AC:L/PR:L/UI:R/S:C/C:L/I:L/A:N                                                                                                                                                                       ┃
┣━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Link for more info ┃ https://ossindex.sonatype.org/vuln/78543c8b-2436-489a-8a43-9a88291114a6?component-type=golang&component-name=github.com%2Fprometheus%2Fprometheus&utm_source=nancy-client&utm_medium=integration&utm_content=0.3.1 ┃
┗━━━━━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Summary                       ┃
┣━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━┫
┃ Audited Dependencies    ┃ 407 ┃
┣━━━━━━━━━━━━━━━━━━━━━━━━━╋━━━━━┫
┃ Vulnerable Dependencies ┃ 1   ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━┻━━━━━┛

cc @bhamail / @DarthHater

This is wild! Thanks for reporting it. Our service works on these types of versions, but being this is a mixed case it will be interesting to figure out how to solve it. I'll forward to a few people and see what we can do. For now, you could add this vuln to a .nancy-ignore file in your project if you want to sidestep it.

I think my suggestion at current time is to use the tag itself as a version, and not the commit hash. I think you should be able to do:

replace (
    github.com/prometheus/prometheus => github.com/prometheus/prometheus v2.23.0
)

This should be fine because your go.sum file keeps a ledger of tags to hashes, to make sure the provenance is fine (aka they didn't swap the tag to a new hash).

Thanks a lot! Yeah we simply added a .nancy-ignore to get unblocked for now.

When trying to replace using the tag, go mod tidy is still unhappy:

Using the base path:

replace (
    github.com/prometheus/prometheus => github.com/prometheus/prometheus v2.23.0
)

go.mod:12:2: replace github.com/prometheus/prometheus: version "v2.23.0" invalid: module contains a go.mod file, so major version must be compatible: should be v0 or v1, not v2

Using a /v2 path:

replace (
    github.com/prometheus/prometheus => github.com/prometheus/prometheus/v2 v2.23.0
)

go.mod has non-.../v2 module path "github.com/prometheus/prometheus" (and .../v2/go.mod does not exist) at revision v2.23.0

And replacing an incompatible version (with or without /v2):

replace (
    github.com/prometheus/prometheus => github.com/prometheus/prometheus/v2 v2.23.0+incompatible
)

go.mod: verifying module: github.com/prometheus/prometheus@v2.23.0+incompatible/go.mod: reading https://sum.golang.org/lookup/github.com/prometheus/prometheus@v2.23.0+incompatible: 410 Gone
    server response: not found: github.com/prometheus/prometheus@v2.23.0+incompatible: invalid version: +incompatible suffix not allowed: module contains a go.mod file, so semantic import versioning is required

If you run go get with the version and module, what's it spit into your go.mod? (I'm more or less curious at this point).

The golang ecosystem has gotten a lot better but this stuff is definitely a fun lil headache :)

I tagged a few Sona-friends internally, I'll see if I can gather any information.

I guess it's the same issue which makes go get not work with their versioning scheme. Using the tag:

$ go get github.com/prometheus/prometheus@v2.23.0
go get github.com/prometheus/prometheus@v2.23.0: github.com/prometheus/prometheus@v2.23.0: invalid version: module contains a go.mod file, so major version must be compatible: should be v0 or v1, not v2

Running $ go get github.com/prometheus/prometheus@v0.0.0-20201126101154-26d89b4b0776 updates the require to that version (v0.0.0-20201126101154-26d89b4b0776).

They recommend using the commit hash directly:

$ go get github.com/prometheus/prometheus@26d89b4b0776fe4cd5a3656dfa520f119a375273
go: github.com/prometheus/prometheus 26d89b4b0776fe4cd5a3656dfa520f119a375273 => v1.8.2-0.20201126101154-26d89b4b0776

which updates the require to

require github.com/prometheus/prometheus v1.8.2-0.20201126101154-26d89b4b0776

but which has the same nancy finding.

I think that a) they need to figure out how to get it working properly (seems kinda wacky to not allow someone to get stuff via the actual tag) but I b) know that sometimes that will never happen, :)

I tried running everything locally to see what happens, and I think go mod is getting confused because there are old incompatible versions, and so on and so forth.

I'll see what we can figure out. The service that does lookup for this type of stuff is likely a bit confused encountering a hash where it expects a semver version range (it's smart but not WICKED smart :) )

Totally agreed -- the root issue is clearly not with nancy. If there is something that can be done on the nancy side, that would be fantastic, but I realize it might not make sense to accommodate all these edge cases.

Thanks for looking into it, I'll watch this space :crossed_fingers:

So we looked at this a lil bit, what is happening is since there is a version that exists with a vulnerability, and the range is unbounded (all versions affected prior to this one), it's returning the vuln as a result of that. In THEORY what we could do is:

if the version is a v0.0.0-TIMESTAMP-HASH type pattern, ignore any semver vulns, and check for those specially (which is what we do sort of currently).

It's hard to say if that would be a wonderful solution, because the hashes COULD be between some semver range, it would just be next to impossible to tell. The way we treat these golang vulns with the timestamps is we look between dates to figure out what works.

I'm not sure I understand the current internals enough, so this might be way off:

Two thoughts:

in this particular case, the commit described by the v0.0.0-TIMESTAMP-HASH pattern also has a vx.x.x tag -- would there be a downside to checking for additional tags for this pattern?
in the absence of an additional tag on the same commit, I'd suggest something like the below, but IIUC you already do something like this?

   v1.0.0                   v1.0.1
     |                        |
o----o----o----o----o----o----o----o <---master
               |
     v0.0.0-20200202-123456

then you compare the timestamps to guess that v1.0.0 < v0.0.0-20200202-123456 < v1.0.1?

If that's correct, and the second method doesn't work (which I can understand it might not a lot of the time), then I guess my only suggestion would be to check for other version tags on the commit.

sonatype-nexus-community / nancy

False positives when dependency versioning isn't go semver #205