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

upgrade golang.org/x/net package to fix CVE-2024-45338 #155

Merged
merged 4 commits into from
Feb 18, 2025

Conversation

ewollesen
Copy link
Contributor

My investigation indicates that neither our code, nor our dependencies
use the functions in question, but at the same time, the impact of
upgrading them is minimal, so it feels like the less risky path to
just upgrade and not have to worry.

Intentionally not bumping go-common at this time. The version of
go-common used by shoreline is so old that it does not have the
vulnerability, and upgrading it would require additional shoreline
modifications.

BACK-3351

My investigation indicates that neither our code, nor our dependencies
use the functions in question, but at the same time, the impact of
upgrading them is minimal, so it feels like the less risky path to
just upgrade and not have to worry.

Intentionally not bumping go-common at this time. The version of
go-common used by shoreline is so old that it does not have the
vulnerability, and upgrading it would require additional shoreline
modifications.

BACK-3351
My investigation indicates that neither our code, nor our dependencies
use the functions in question, but at the same time, the impact of
upgrading them is minimal, so it feels like the less risky path to
just upgrade and not have to worry.

This seems to be related to the issue that caused CVE-2024-45338, and
I think the fix is ultimately the same, but snyk insists that the old
version of golang-jwt/jwt is susceptible, despite us having upgraded
to x/net/html package previously. Perhaps they cut and pasted some
code or something. Either way, let's fix it up.

BACK-3351
@ewollesen
Copy link
Contributor Author

The snyk vulnerability has no remediation at present (awaiting a release from upstream), so we'll have to bypass that for now.

The latest version isn't compatible with go 1.22. Plus, pinning
versions is a good thing so interruptions like this one in the build
process don't happen.
@@ -5,7 +5,7 @@ RUN adduser -D tidepool && \
apk add --no-cache git gcc musl-dev && \
chown -R tidepool /go/src/github.com/tidepool-org/shoreline
USER tidepool
RUN go install github.com/cosmtrek/air@latest
RUN go install github.com/air-verse/air@v1.52.2
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the version used in hydrophone

Copy link
Contributor

@jh-bate jh-bate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - one comment

@@ -28,8 +28,8 @@ before_install:
addons:
apt:
sources:
- sourceline: 'deb https://repo.mongodb.org/apt/ubuntu jammy/mongodb-org/7.0 multiverse'
key_url: 'https://pgp.mongodb.com/server-7.0.asc'
- sourceline: 'deb https://repo.mongodb.org/apt/ubuntu jammy/mongodb-org/6.0 multiverse'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is for consistency with other services?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

My understanding is, that even though we were using the 7.0 repo previously, we were using 6.x releases of MongoDB, which matches current prod. At some point the 6.x releases of MongoDB stopped being packaged in the 7.0 repo (I don't know why they ever were), and so we've had to make this adjustment. This isn't the first package to see this change, we're just updating them as we find them.

@ewollesen ewollesen merged commit e12400d into master Feb 18, 2025
2 of 3 checks passed
@ewollesen ewollesen deleted the eric-back-3351 branch February 18, 2025 19:26
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 this pull request may close these issues.

2 participants