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

fix: CVE-2023-7104(github.com/mattn/go-sqlite3) #829

Merged
merged 1 commit into from
Apr 12, 2024

Conversation

AjayJagan
Copy link
Contributor

@AjayJagan AjayJagan commented Jan 30, 2024

This PR is intended to fix CVE-2023-7104.

Description

It fixes the following CVE

How Has This Been Tested?

Tried to run the operator and everything works well.

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

Copy link

openshift-ci bot commented Jan 30, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@AjayJagan
Copy link
Contributor Author

AjayJagan commented Jan 30, 2024

after looking at the libraries to update, it seems that this: github.com/operator-framework/operator-lifecycle-manager@v0.26.0(This is the latest version)
is one of the libraries which uses the above versions which has the cves.
So this pr is kind of a temporary fix.
Either we can use this or we can wait until they update their library.
let me know what you think @zdtsw @VaishnaviHire :)

@VaishnaviHire
Copy link
Member

Updating to github.com/operator-framework/operator-lifecycle-manager@v0.26.0 introduces other critical issues in synk

@@ -101,7 +101,9 @@ require (

replace (
github.com/go-yaml/yaml => github.com/go-yaml/yaml v2.2.8+incompatible
github.com/mattn/go-sqlite3 => github.com/mattn/go-sqlite3 v1.14.18
github.com/tektoncd/pipeline => github.com/tektoncd/pipeline v0.12.0
Copy link
Member

Choose a reason for hiding this comment

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

Need to run go mod tidy to update go.sum

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have run the go mod tidy but the issue is that library is not present in our go.sum. Instead it is being used by one of the libraries we use and that is the reason why we see this 😅 .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

since it is a dependency of another library, I guess we cannot do much unless they update it. So I though we can temporarily fix it using the replace 😅 . But if this is not the right way to do so, we should ignore the cve until there is an update from the other library's side. Please let me know what you think

Copy link
Member

Choose a reason for hiding this comment

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

Yes I agree, but we still would have some updates in go.sum..right? Given we are introducing additional packages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to a go mod tidy, and there is no update to go.sum. I have no idea why 😅

Copy link
Member

Choose a reason for hiding this comment

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

hum, when i used your go.mod from this PR , i have a lot cleaned up in go.sum

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried again :( no updates for me. Am I missing something while doing go mod tidy

Copy link
Member

Choose a reason for hiding this comment

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

@AjayJagan You can try cleaning up the cache once

 go ​clean -modcache

Copy link
Contributor Author

@AjayJagan AjayJagan Feb 20, 2024

Choose a reason for hiding this comment

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

done, still the same go.sum file 😅

@AjayJagan
Copy link
Contributor Author

Updating to github.com/operator-framework/operator-lifecycle-manager@v0.26.0 introduces other critical issues in synk

@VaishnaviHire but we are already on v0.26.0 right? In the incubation branch itself?

@AjayJagan
Copy link
Contributor Author

/retest-required

@AjayJagan
Copy link
Contributor Author

/retest

@zdtsw
Copy link
Member

zdtsw commented Apr 8, 2024

/test opendatahub-operator-e2e

@AjayJagan
Copy link
Contributor Author

looks like e2e fails, I will take a look

@zdtsw
Copy link
Member

zdtsw commented Apr 8, 2024

/retest

@AjayJagan
Copy link
Contributor Author

@VaishnaviHire , I have used replaces on the grpc library. But looks like even after updating, it does not fix the sqllite version. So I updated grpc+sqllite libraries. I tested the operator manually and also ran e2e tests. Everything seems to work well. Please let me know if this is okay :)

@AjayJagan AjayJagan changed the title fix HIGH cves fix: CVE-2023-7104(github.com/mattn/go-sqlite3) Apr 9, 2024
Copy link
Member

@VaishnaviHire VaishnaviHire left a comment

Choose a reason for hiding this comment

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

/lgtm

Copy link

openshift-ci bot commented Apr 12, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: VaishnaviHire

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

openshift-ci bot commented Apr 12, 2024

New changes are detected. LGTM label has been removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants