search

getsentry / sentry-go

The official Go SDK for Sentry (sentry.io)
https://docs.sentry.io/platforms/go/
MIT License
906 stars 211 forks source link

go.sum contains reference (via Echo framework integration) to module affected by CVE #376

Closed erlichmen closed 2 years ago

erlichmen commented 3 years ago

sentry-go is using a rather old version of the echo module which uses a JWT module that has a CVE (the latest (since July 2020) echo module has resolve that). This causes our repo to raise a flag and that makes the chain of brass itchy.

The only solution right now is to remove sentry and we really don't want it.

bww commented 3 years ago

Same problem here, we don't want to fork your client and we can't just ignore this. It's fairly surprising for this not to have been addressed by now, especially given what your product does. Don't you guys get the Dependabot alerts about this?

Version 4.5.0 of echo has fixed the problem, you should be able to just bump the version: https://github.com/labstack/echo/blob/v4.5.0/go.sum

rhcarvalho commented 2 years ago

This is somewhat related to #156.

The github.com/getsentry/sentry-go package itself has no external dependencies, but the integration packages (e.g. github.com/getsentry/sentry-go/echo) do bring in module dependencies into github.com/getsentry/sentry-go/go.mod.

The echo framework and the jwt-go package referenced in the CVE will only be in a built binary if package main (in the main module) is using echo, and then its go.mod file is the one dictating the version of echo (and other dependencies) that will be used (not sentry-go's go.mod/go.sum).

Sentry SDKs generally avoid forcing people upgrading to keep broad compatibility, but in this case I think we can bump the dependency as a remedy to those getting dependabot notifications.

tylerd-canva commented 2 years ago

Ah, actually that explains the problem I had figuring out where my own remaining dependency on github.com/dgrijalva/jwt-go was coming from. go mod why insisted I didn't need it, but it remained in my go.sum file. I had to resort to go mod graph to find it.

Given the complexity around this I've gone ahead and worked around the problem on my end by adding an exclude statement to my go.mod.

// See https://github.com/getsentry/sentry-go/issues/376
exclude github.com/labstack/echo/v4 v4.1.11

I'll leave it up to the Sentry folks to accept or decline my PR but anyone facing this issue should be able to unblock themselves.

rhcarvalho commented 2 years ago

This causes our repo to raise a flag and that makes the chain of brass itchy.

@erlichmen what tool are you using that flags this as an issue?

I would like to see if we can make some tools smarter -- notice that go.sum is not a lock file, but some tools treat it like so. The go.sum file is a database of module cryptographic hashes, and the modules/versions listed there are not necessarily part of your build dependencies.

From the main package directory, use go list all to see the build dependencies (and prior to Go 1.16 that also includes the test dependencies of all dependencies).

Dependabot alerts about this

@bww do you have an example of alert/PR in a public repo? I'd love to use that to open an issue in https://github.com/dependabot/dependabot-core.

It used to be the case (based on this comment from a core Dependabot developer) that Dependabot dit not have access to the complete source code required to run go list ./... to determine potential outdated or problematic dependencies.

But since that issue was closed and they run go mod tidy maybe it would also be possible to determine actual dependencies using go list instead of go.mod/go.sum. (Reference: https://github.com/dependabot/dependabot-core/tree/main/go_modules)

phmx commented 2 years ago

Could you please bump the sentry-go release tag to incorporate this update?