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

Update uuid dependency from satori/go.uuid to gofrs/uuid #1052

Open
Technerder opened this issue Jul 26, 2021 · 10 comments
Open

Update uuid dependency from satori/go.uuid to gofrs/uuid #1052

Technerder opened this issue Jul 26, 2021 · 10 comments

Comments

@Technerder
Copy link

Similar to my previous issue, satori/go.uuid included here is unmaintained and suffers from a 3 year old CVE-2021-3538 which seems to be flagged on Snyk. Upgrading to a maintained fork such as satori/go.uuid (as suggested here) should fix this.

@jackc
Copy link
Owner

jackc commented Jul 31, 2021

These are really frustrating. There is no production dependency on the insecure library. It was switched to the gofrs fork 2 years ago (jackc/pgtype@cf8fe4a).

pgx depends on pgtype. But pgtype tests use pgx to integration test with a real database. This means the pgtype tests can only depend on the 2nd latest pgx which can only depend on the 2nd latest pgtype whose tests can only depend on the 3rd latest pgx, etc... That's messy, but integration testing the type mapping is really important.

It really seems unreasonable that Go modules is recursively including test dependencies of test dependencies and the same for security scanners.

@Technerder
Copy link
Author

Yeah, its rather unfortunate that this is an issue. Hopefully the proposal you suggested gets implemented.

@akshaybharambe14
Copy link

akshaybharambe14 commented Aug 14, 2021

Not sure, but can we have a separate repository OR separate module in the same pgtype repository that takes care of integration tests?

naveensrinivasan added a commit to naveensrinivasan/lnd that referenced this issue Oct 1, 2021
Fix the satori/go.uuid reference to avoid the CVE.
More information jackc/pgx#1052
satori/go.uuid#75
satori/go.uuid#73
@Technerder
Copy link
Author

Is this issue now resolved?

@jackc
Copy link
Owner

jackc commented May 13, 2022

Is this issue now resolved?

It is on v5 anyway.

@ne0z
Copy link

ne0z commented Jul 12, 2022

Hi there, I got the same issue. I agree with @jackc that there is no production dependency on the insecure library. It was switched to the gofrs fork 2 years ago.

After a quick investigation, the satori module come from go.sum which was not removed completely either even after switching to gofrs. So, Satori in the go.sum is like junk that makes SCA tools like Snyk show false positives.

➜  pgtype git:(master) go mod graph | grep satori
github.com/jackc/pgtype@v0.0.0-20190828014616-a8802b16cc59 github.com/satori/go.uuid@v1.2.0
github.com/jackc/pgx/v4@v4.0.0-pre1.0.20190824185557-6972a5742186 github.com/satori/go.uuid@v1.2.0
github.com/jackc/pgtype@v0.0.0-20190824184912-ab885b375b90 github.com/satori/go.uuid@v1.2.0
github.com/jackc/pgx/v4@v4.0.0-20190421002000-1b8f0016e912 github.com/satori/go.uuid@v1.2.0
github.com/jackc/pgtype@v0.0.0-20190421001408-4ed0de4755e0 github.com/satori/go.uuid@v1.2.0
github.com/jackc/pgx/v4@v4.0.0-20190420224344-cc3461e65d96 github.com/satori/go.uuid@v1.2.0
cd ~/go/pkg/mod/github.com/jackc
➜  grep -r "satori"
./pgmock@v0.0.0-20210724152146-4ad1a8207f65/go.sum:github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0=
./pgconn@v1.9.1-0.20210724152538-d89c8390a530/go.sum:github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0=
./pgx/v4@v4.12.1-0.20210724153913-640aa07df17c/go.sum:github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0=

So we just need to delete satori module from go.sum on these repository in the same time:

  • github.com/jackc/pgtype
  • github.com/jackc/pgx/v4
  • github.com/jackc/pgconn
  • github.com/jackc/pgmock

@jackc
Copy link
Owner

jackc commented Jul 16, 2022

@ne0z Unfortunately, I don't think it is so simple. I tried the change on jackc/pgconn#123. But the next run of go mod tidy returns the reference.

I vaguely recall fighting with this on a different dependency and the solution was something like delete all the tests in all the related packages and commit in each package. Run go mod tidy and commit in each package. Update the dependencies to the no test version, go mod tidy, and commit. Revert the commits that removed the tests. Or something like that anyway -- it was a real hassle.

@ne0z
Copy link

ne0z commented Jul 17, 2022

Hi @jackc, as I mentioned above, you can't just try jackc/pgconn#123 alone, because that satori module is mentioned on go.sum on your multiple repositories. So you need to delete the satori module on these repositories same time (or merge same time without running go mod tidy):
github.com/jackc/pgtype
github.com/jackc/pgx/v4
github.com/jackc/pgconn
github.com/jackc/pgmock

@ne0z
Copy link

ne0z commented Jul 17, 2022

Hi @jackc I think you are right, deleting all the tests in all the related packages and committing in each package manually was a real hassle.

@geoeee
Copy link

geoeee commented Feb 21, 2023

In satori repo, use the latest master branch can resolve the CVE issue but the api is changed to return (UUID, error)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants