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

Vulnerability in github.com/satori/go.uuid #51

Closed
princjef opened this issue May 14, 2021 · 3 comments · Fixed by #69
Closed

Vulnerability in github.com/satori/go.uuid #51

princjef opened this issue May 14, 2021 · 3 comments · Fixed by #69
Assignees

Comments

@princjef
Copy link
Member

One of the dependencies in this package (github.com/satori/go.uuid) has a vulnerability that dates back quite a while with no published version that has a fix. The repository for that package appears to be abandoned. Here's the CVE: https://snyk.io/vuln/SNYK-GOLANG-GITHUBCOMSATORIGOUUID-72488

Is it possible to upgrade to a different package? I see that there is also a dependency on github.com/google/uuid here. As another reference point, the Azure SDK for Golang chose to move to github.com/gofrs/uuid when faced with the same issue: Azure/azure-sdk-for-go#14283

@element-of-surprise
Copy link
Member

I had a brief look at this today and I don't believe we suffer from the vulnerability. The vulerability seems based around not having enough random bytes when generating a UUID allowing an ID to be guessed. We don't generate UUIDs with satori's package.

This is used in the ingestion code if satori's package is in use to read an existing UUID that we did not generate and convert it to Google's UUID format. I'm not sure why this exists there, so I'm going to have to go back and look if this was a customer need or something like that. If we can strip it, we will.

I looked over at the one that SDK is using. We are unlikely to migrate to it. Google's version is based on Paul Borman's uuid package. They beat on his for years and then used it for the base of Google's official one. It is used by all the largest projects and I have more faith in it. It also has more recent updates (though not a ton).

Will update this when I figure out why we have this in there at all.

@element-of-surprise element-of-surprise self-assigned this May 17, 2021
@yogilad
Copy link
Contributor

yogilad commented May 18, 2021

Hi,

Satori UUID is a dependency of Azure Storage Tables in Go SDK.
We only use it to convert read UIDs from Satori to Google UIDs.
We do not generate UIDs with it as the table values are fed from google UID GUIDs.

// Status.go
import (
	"github.com/google/uuid"
	storageuid "github.com/satori/go.uuid"
)
...
case storageuid.UUID:
		uid, err := uuid.ParseBytes(x.(storageuid.UUID).Bytes())
		if err != nil {
			return uuid.Nil
		}

@element-of-surprise
Copy link
Member

It looks like storage's newer SDK uses the gofrs uuid package (shakes head). We could just up our dependency on them and switch out to the new one to eliminate this package. It just helps ensure we never use it in the future by accident.

Another option is the author works for SRE, currently on leave. We could see if he is willing to apply the fix, which is pretty trivial.

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

Successfully merging a pull request may close this issue.

4 participants