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 to Snakeyaml 2.0 (resolves CVE-2022-1471) #390

Merged
merged 1 commit into from
Feb 26, 2023

Conversation

pjfanning
Copy link
Member

@pjfanning pjfanning commented Feb 26, 2023

CVE fix: https://bitbucket.org/snakeyaml/snakeyaml/issues/561/cve-2022-1471-vulnerability-in
(for https://nvd.nist.gov/vuln/detail/CVE-2022-1471)

  • tests pass alright
  • may be a bit risky to put into jackson 2.14 (snakeyaml 2.0 is a major release)

@cowtowncoder
Copy link
Member

Hmmmh. Might be bit risky even for 2.15, no?

I am guessing tho that major upgrade here is not due to API change but for... marketing?
Do you have more info on this?

@pjfanning
Copy link
Member Author

Snakeyaml was refactored a bit to make it more secure by default. Refactor doesn't seem to break anything Jackson wise. It does affect other code bases that use more of the snakeyaml APIs

My view is that Jackson upgrading is probably ok. And means we avoid the CVE marker on our transitive dependencies. Users who use this module and another snakeyaml dependent lib may run into issues with the other lib not yet up taking the latest snakeyaml.

@cowtowncoder cowtowncoder changed the title snakeyaml 2.0 Upgrade to Snakeyaml 2.0 (resolves CVE-2022-1471) Feb 26, 2023
@cowtowncoder cowtowncoder added yaml Issue related to YAML format backend 2.15 Fix or feature targeted at 2.15 release labels Feb 26, 2023
@cowtowncoder
Copy link
Member

Ah. And 2.0 version bump is not for marketing but due to defaults change which is behavioral.
And since SnakeYAML does not use patch numbers -- that is, only uses 2 digits, it is not "small enough" to qualify for 1.34 -- it makes sense.

I'll merge this and then need to add some verbiage in 2.15 release notes: I agree it's unlikely this should cause significant breakage compared to, say, earlier introduction of processing limits wrt input doc size.

Thank you for staying on top of this @pjfanning: those transitive dep CVEs are unfortunate for users.

@cowtowncoder cowtowncoder modified the milestones: 2.10, 2.15.0 Feb 26, 2023
@cowtowncoder cowtowncoder merged commit 3a433d8 into FasterXML:2.15 Feb 26, 2023
@pjfanning pjfanning deleted the snakeyaml-2.0 branch February 26, 2023 19:07
cowtowncoder added a commit that referenced this pull request Feb 26, 2023
@cowtowncoder
Copy link
Member

@pjfanning
Copy link
Member Author

Merged, added a note on

https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.15

The release note says upgrade to Snakeyaml 2.0 from 2.13 - I'm not sure what the 2.13 means.

One thing that I've run into is that Spring framework seems to use snakeyaml to read config files and the Spring framework code seems to use APIs that break with Snakeyaml 2.0. See the test failures in https://pipelines.actions.githubusercontent.com/serviceHosts/2b2c8c25-f923-4d14-bc54-07f0604e4241/_apis/pipelines/1/runs/19578/signedlogcontent/2?urlExpires=2023-02-26T14%3A37%3A10.9289750Z&urlSigningMethod=HMACV1&urlSignature=sV%2ByS%2BFcZ9mx2OAGZDj9jvplw%2BRYYwVAHctlPnHzBRU%3D

I still think that anyone who uses a lib that breaks with snakeyaml 2.0 can choose to hold off the Jackson upgrade until the other lib is fixed up. Everyone else benefits from using the more secure snakeyaml release.

@cowtowncoder
Copy link
Member

Ok that was my fat fingering; was to be to 1.33. Thanks, will fix.

Hmmmh. Spring breakage, if it cannot be solved from their side, is something that would make me consider reverting this upgrade -- considering that one can override snakeyaml dependency for Jackson for newer version easily.
I guess technically one can also downgrade it but that has the issue of CVE reporting.

So I guess we need to keep this in mind for 2.15.0 release candidate phase and see what makes most sense.

@pjfanning
Copy link
Member Author

I looked a bit closer at the Apache Linkis issue and they are using a fairly old bundle of Spring jars.

It seems like newer Spring versions may be better off. I looked at the Spring code and it has been adjusted to work with a newer 1.x version of snakeyaml.

It looks like some deprecated methods and constructors were removed in snakeyaml 2.0.

@cowtowncoder
Copy link
Member

@pjfanning Ok thank you. I think we can deal with this as part of pre-release cycle then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.15 Fix or feature targeted at 2.15 release yaml Issue related to YAML format backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants