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): remove CVE-2022-22965 in springframework libraries #1091

Merged
merged 1 commit into from
Aug 18, 2023

Conversation

edgarulg
Copy link
Contributor

Spinnaker 1.30.x is being affected by the vulnerability CVE-2022-22965 coming from spring-framework dependencies. This is a critical vulnerability we want to fix because it allow to attackers to make remote code execution. Here you will find more info about the vulnerability: https://nvd.nist.gov/vuln/detail/cve-2022-22965

In Spinnaker 1.30.x we use the spring-boot-dependencies:2.4.13 and it brings org.springframework:spring-core:5.3.13, org.springframework:spring-beans:5.3.13 & org.springframework:spring-webmvc:5.3.13. These libraries contains the vulnerability in mention. Here you will find the vulnerabilities in spring-boot-dependencies:2.4.13 https://mvnrepository.com/artifact/org.springframework.boot/spring-boot/2.4.13

spring-boot-dependencies was upgraded in Spinnaker 1.31.x and master and Spinnaker 1.30.x has the latest version of spring-boot-dependencies:2.4.x and it is risky to upgrade to a newest minor version in Spinnaker 1.30.x. It also break our rule to don't backport features.

Talking a little bit more about my changes. I included constraint statements to force the version 5.3.18 of org.springframework:spring-core, org.springframework:spring-beans & org.springframework:spring-webmvc. This change force all our OSS services to use 5.3.18 without making risky upgrades in Spring because we upgrade patch version. Here you will find the vulnerability fixed: https://mvnrepository.com/artifact/org.springframework/spring-core/5.3.18

I tested my changes against a few OSS services and I didn't see breaking changes as part of this. Also I generated a custom image and then analyze it with a vulnerability scan tool and the CVE was gone.

Copy link
Contributor

@dbyron-sf dbyron-sf left a comment

Choose a reason for hiding this comment

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

LGTM, but we're pretty backed up with changes to 1.30.x that I'd like to release before we merge this.

@edgarulg
Copy link
Contributor Author

@dbyron-sf yea sure, I heard it from @link108 as well. I added the do not merge label until all changes you want to merge first are done.
Another question that comes to my mind is Do you think would be good to stay in 5.3.18 instead of use a latest version? In my opinion I prefer stay stable and reduce risk as much as possible. So I would like to stay in 5.3.18

@dbyron-sf
Copy link
Contributor

Yeah, I'm all for the smallest change that resolves the CVE, so 5.3.18 works for me.

@dbyron-sf
Copy link
Contributor

I'm not sure who is on the hook to get the next 1.29.x and 1.30.x out the door, but if you have time, it would be a big help.

@edgarulg
Copy link
Contributor Author

edgarulg commented Aug 15, 2023

@link108 is going to help with the 1.29.x and 1.30.x releases. FYI @dbyron-sf

@link108
Copy link
Member

link108 commented Aug 18, 2023

I just finished up releasing 1.29.6 and 1.30.3 and will remove the do not merge label 👍
cc @edgarulg @dbyron-sf

@edgarulg edgarulg added the ready to merge Approved and ready for merge label Aug 18, 2023
@mergify mergify bot added the auto merged label Aug 18, 2023
@mergify mergify bot merged commit 6b5fc44 into spinnaker:release-1.30.x Aug 18, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants