spring-projects / spring-boot

Spring Boot helps you to create Spring-powered, production-grade applications and services with absolute minimum fuss.
https://spring.io/projects/spring-boot
Apache License 2.0
75.17k stars 40.68k forks source link

CVE-2022-25857 - Upgrade to SnakeYAML 1.31 #32221

Closed bclozel closed 2 years ago

bclozel commented 2 years ago

CVE-2022-25857 has been reported against the SnakeYaml project. This issue upgrades SnakeYaml to 1.31 for Spring Boot 3.0.0.

This CVE can make applications vulnerable to DoS attacks, given the Yaml parser is used to parse untrusted input. Most Spring Boot applications use this library to parse their own application.yml configuration file, which is considered as safe. If an attacker could change application.yml to exploit this vulnerability, they could cause much more damage than a DoS by just changing the properties, or by reading secrets.

The Spring Boot policy for upgrading third party dependencies in our dependency management prevents us from upgrading this version in maintenance branches, 2.6.x and 2.7.x. Doing so would expose developers to possible behavior or API changes that would disrupt their application. We've discussed the possibility of making an exception to this policy, but this case happened in the past already with SnakeYaml 1.26 (see #20366); so far we don't see a reason to do so and we expect libraries maintainers to release patch versions for CVE fixes.

If your 2.6.x or 2.7.x application is using SnakeYaml to decode untrusted Yaml, for example from a web controller, you should override the SnakeYAML version property (snakeyaml.version) as soon as possible in your Gradle or Maven build.

bclozel commented 2 years ago

Update: we've just pushed a fix for broken timestamp/dates handling with SnakeYaml 1.31 in the upcoming Spring Boot 2.6.x and 2.7.x. We're still defaulting to 1.29 and 1.30 in those versions, but unlocking the possibility to use SnakeYaml 1.31 at runtime; see #32228.

bclozel commented 2 years ago

@nytro77 thanks I've edited my comment. Yes we'll stick to 1.30 in 2.7.x for the same reason.

nytro77 commented 2 years ago

@bclozel Ah ok. Just deleted my comment because i thought my eyes tricked me :) Great thanks

asomov commented 2 years ago

It is sad, that false positives have such an influence.

robert-gdv commented 2 years ago

@bclozel Would you include an update to a potential Version 1.30.1 of snakeyaml, that fixes only same bugs in 2.7.x ? I am trying to convince the maintainer of snakeyaml to release such a hotfix.

My issue is, that I support dozens of projects where this vulnerability popped up. Unfortunately I can't mark it in general as "false positive". Instead I have to check every project, that it doesn't process external yamls. I assume that there a thousands of teams around the world doing the same thing.

asomov commented 2 years ago

@bclozel what are the bugs ?

wilkinsona commented 2 years ago

IMO, the root cause of the problem is the CVE database and security scanning throwing up false positives. Trying to convince an open source maintainer to take on an additional maintenance burden due to that doesn't feel right to me.

You can use SnakeYAML 1.31 right now with Spring Boot 2.6 and 2.7 with one small caveat. Due to a change in a method that Spring Boot overrides, some custom handling of timestamp-like values is lost. Consider the following YAML:

example: 2015-01-28

With SnakeYAML 1.30 and earlier, Spring Boot will successfully customize SnakeYAML so that the value is left as-is. With SnakeYAML 1.31 this customization does not succeed and it is coerced into a java.util.Date. You can work around this change in behaviour by quoting the value, thereby ensuring that it's left as-is:

example: '2015-01-28'

The forthcoming Spring Boot 2.6.x and 2.7.x releases adapt to the changes in SnakeYAML 1.31 so that this quoting isn't necessary (but won't do any harm).

asomov commented 2 years ago

@wilkinsona can you please provide the method which was changed ? We do our best to keep backwards compatible changes

asomov commented 2 years ago

I can help to integrate the latest version of SnakeYAML into Spring Boot !

bclozel commented 2 years ago

@asomov it's not a bug and I wouldn't blame SnakeYaml for breaking backwards compatibility. We're subclassing Resolver in order to skip the resolver registered by default for Tag.TIMESTAMP values. We should really ask for a proper way to disable one of the built-in resolvers instead of subclassing here - something that would be less brittle on our side. See https://github.com/spring-projects/spring-boot/commit/0789dd0eb1c6f55e2cc70b9ead98f911b0031b6c#diff-07741e308f54bc7fc66aabb0a1594c1ff8a9785103fb8cdf4c930ad3b44ed2c6

wilkinsona commented 2 years ago

Thanks, @asomov.

We use a custom Resolver subclass to disable the timestamp handling that I described above. In SnakeYAML 1.31 the introduction of the addImplicitResolver(Tag tag, Pattern regexp, String first, int limit) method rendered our override ineffective. @bclozel made a change to adapt to this in https://github.com/spring-projects/spring-boot/commit/724f9ebf7107d99ac9bae4204b4f7c6444a49e03. We now use a different Resolver subclass depending on whether SnakeYAML < 1.31 or >= 1.31 is being used at runtime.

I can help to integrate the latest version of SnakeYAML into Spring Boot

Thanks very much for the offer. I think we've already managed to do this. Spring Boot 2.6.x and 2.7.x snapshots are fully compatible with SnakeYAML 1.31 (while continuing to use 1.29 and 1.30 respectively by default). Spring Boot 3.0 snapshots have upgraded to SnakeYAML 1.31.

asomov commented 2 years ago

Thank you. I see how myself and others suffer from the very low quality of CVE (whatever it is). Our fix made your life harder. I am sorry. Actually, it is no fix, it pretends to be a fix, but for the low quality tooling this is enough. I am very disappointed.

asomov commented 2 years ago

By the way, may be Spring Boot can consider the migration to the SnakeYAML Engine which does not have the special treatment of 2022-09-06 value. I am here to support the transition

asomov commented 2 years ago

If I knew that I may create such an issue for Spring Boot, I would not apply this change. The value of it is negligible, the consequence it awful. @wilkinsona should this change be reverted ? Or this is too late ?

bclozel commented 2 years ago

@asomov you don't need to revert anything, it's all taken care of on our side now and this will be released soon.

xuekvm commented 2 years ago

Hi, our team project needs to address cve-upgrade snakeyaml to 1.31. I specify the snakeyaml version in the build.gradle file as below: ext['snakeyaml.version'] = '1.31' the project uses spring boot 2.5. So I was wondering if we use spring boot 2.5, can we use snakeyaml 1.31 as way above. What I want to confirm is if snakeyaml is related to spring boot. Can we use a lower version of spring boot(2.5), while use a higher version of snakeyaml(1.31) Thanks in advance.

snicoll commented 2 years ago

@xuekvm Spring Boot 2.5 is out of OSS support so you'll have to upgrade to a supported version first. We're aware of one problem with 1.31 that we've fixed in 2.6.12, see #32228.

majdak965 commented 2 years ago

Hello , after adding this property 1.31</snakeyaml.version> , when showing the effective pom , I saw always the ancient version , the version of spring boot is 2.6.9

org.yaml snakeyaml 1.30

Is it normal? Can someone helps me please.

wilkinsona commented 2 years ago

Spring Boot 2.6.x uses SnakeYAML 1.29 by default so something other than Spring Boot is controlling its version and setting it to 1.30. If you'd like some help with that, please post a question on Stack Overflow and include a pom that reproduces the problem in your question.

abegum123 commented 2 years ago

Hi, I am using springboot version 2.7.1 and upgrading snake yml dependency to 1.32 is breaking the code base due to date fields in yml files. could you please suggest what spring boot version we need to upgrade to have these compatibility issue fixed? I don’t find the 2.6.12 version in maven repo.

bclozel commented 2 years ago

@abegum123 this is already fixed and to be shipped with Spring Boot 2.6.12 and 2.7.4, see #32228

Both releases are scheduled next week.

abegum123 commented 2 years ago

sure, Thank you @bclozel

wilkinsona commented 2 years ago

@abegum123 In the meantime, quoting the date fields in your yaml files will work around the problem. This causes SnakeYAML to leave the values as strings. Spring Boot can then perform its usual conversion from a string to the required type.

abegum123 commented 2 years ago

yes, I tried and it worked. But We have 11 services and changing in all these places will have more impact on the application.

xuekvm commented 2 years ago

@majdak965 Hi, I face the similar issue. We use spring boot 2.5, and want to upgrade snakeyaml to 1.31. Somehow, it can not be upgraded successfully. I tried to upgrade it to 1.30, and it seems to work. But for 1.31, it seems it can not be upgraded successfully. I am trying to figure out what other things controlling snakeyaml version as @wilkinsona suggested, but haven't found it. So ask if you resolve it? Thanks.

xuekvm commented 2 years ago

@wilkinsona thanks for your suggestion. You mentioned maybe some other things controlling snakeyaml, and force set it to 1.30. Is the snakeyaml also controlled by others besides Spring? I think snakeyaml is from Spring, so it should be only controlled by Spring, right?(correct me if I am wrong). Since there are many dependencies in the build.gradle file, and not easy to figure out what else controlling snakeyaml. Can you please specify what others may affect snakeyaml version? I use ext['snakeyaml.version'] = '1.31' to override it, but not able to upgrade to 1.31. Thanks.

majdak965 commented 2 years ago

@xuekvm , In my last comment, I made a mistake as @wilkinsona said the version that was used by spring boot 2.6.9 is snakeyaml 1.29 When showing my dependency tree of my project, there is only spring boot that used snakeyaml So, to resolve this problem, I added the dependecy of snakeyaml to my list of dependencies to force the 1.31 version image

snicoll commented 2 years ago

In the meantime, SnakeYAML 1.32 is out so the dependency upgrade is superseded by https://github.com/spring-projects/spring-boot/issues/32386

asomov commented 2 years ago

@wilkinsona I provided a better solution for the resolver: https://github.com/spring-projects/spring-boot/pull/32464

yairkukielkazunzunegui commented 2 years ago

Hi, although in the release notes of spring boot 2.7.4 it states that the dependency snakeyaml was updated to version 1.31, the package spring-boot-starter 2.7.4 is still depending on snakeyaml 1.30.

Should this issue be re-opened?

snicoll commented 2 years ago

@yairkukielkazunzunegui the release notes does not state such thing. Where do you see a dependency upgrade to snakeyaml 1.31? We upgraded to 1.32 in Spring Boot 3. We can't upgrade to a new major in a maintenance of Spring Boot but we've made sure you can if necessary, see https://github.com/spring-projects/spring-boot/issues/32228

robert-gdv commented 2 years ago

I get the impression, that Fuzzy Scanning is currently revealing a lot of issues with snakeyaml, that need to be fixed (+bookkeeping to inform, that they are fixed). That will probably take some time.

If anyone is using snakeyaml for untrusted data should consider helping the snakeyaml team or use another tool. For the majority, who uses it just for their own configuration (or not at all), the issues with snakeyaml can be ignored, because it can be treated as trusted code.

Rushing snakeyaml updates now won't help those, who have to maintain alarms from dependency security scanner. I expect some updates of snakeyaml in the near future.

yairkukielkazunzunegui commented 2 years ago

sorry @snicoll, my bad. I was refering to this line

YAML timestamps not handled properly with SnakeYaml 1.31 https://github.com/spring-projects/spring-boot/issues/32229

And I assumed that snakeyaml was bumped to 1.31. Thanks for clarifying!

asomov commented 2 years ago

Dear @robert-gdv, unfortunately, you re-distribute the information which is partially confusing, partially just wrong. I would like to clarify.

  1. Fuzzy Scanning is currently NOT revealing a lot of issues with snakeyaml. There are a few which are easily solved with proper configuration of the parser. There is probably one which may still valid. If you think you are correct, please provide the list and we can go one-by-one
  2. Despite my call to show a use case when the parser has to take untrusted input without possibility for very basic sanitization, NO SINGLE real use case was provided. Please help me to find one
  3. The low quality tooling (like DependencyChecker) has hundreds (!!!) of issues, many of which are about false positives. They do not fix them. Why should we bother about the quality if they do not ???
  4. The low quality tooling does not check the most important part - the context. So they blindly complain about anything. Noise.
  5. There is nothing to fix in SnakeYAML. Otherwise please show what exactly.
  6. You cannot simple "use another tool" as you say. All the parsers have to follow the specification and it requires to support data structues which can be misused by a potential attacker.
  7. Please calify this statement "I expect some updates of snakeyaml in the near future." - what exactly you expect ?
npolovnikov commented 1 year ago

image

Hi! I updated to 2.7.4 But the dependency still remains, what am I doing wrong?

wilkinsona commented 1 year ago

@npolovnikov The default version of SnakeYAML hasn't change in Spring Boot 2.7. Please read the opening comment in this issue. It describes when you should consider manually overriding the SnakeYAML version and how to do so.

robert-gdv commented 1 year ago

Dear @robert-gdv, unfortunately, you re-distribute the information which is partially confusing, partially just wrong. I would like to clarify.

Dear @asomov , I deeply respect the time you spend for the project snakeyaml. I am also not happy, that every potential DoS gets a CVE score 7+ I am in the position, where I need to discuss frequently, why those findings are NOT relevant for one of our projects. So I share your view. Nevertheless the findings are there and cost a lot of time to manage downsteam (with management). So I prefer to close them at the source.

  1. Fuzzy Scanning is currently NOT revealing a lot of issues with snakeyaml. There are a few which are easily solved with proper configuration of the parser. There is probably one which may still valid. If you think you are correct, please provide the list and we can go one-by-one

I am sure you know OSS-Fuzz, which seems to be a source of some of the Snakeyaml 1.30 findings - although not directly visible there

  1. Despite my call to show a use case when the parser has to take untrusted input without possibility for very basic sanitization, NO SINGLE real use case was provided. Please help me to find one

As already pointed out, snakeyaml is responsible for yaml sanitization. Otherwise another yaml-parser would need to be build duplicating half of your work.

  1. The low quality tooling (like DependencyChecker) has hundreds (!!!) of issues, many of which are about false positives. They do not fix them. Why should we bother about the quality if they do not ???

Whataboutism Besides that: As you said, the Context is important. DependencyChecker is working with trusted input (pom.xml) and is only used internally. It is normally not even installed on a production system.

  1. The low quality tooling does not check the most important part - the context. So they blindly complain about anything. Noise.

I agree. Unfortunately as long as the Context is not guaranteed, someone could have the idea to use yaml files from users (Example: gitlab-ci) and use snakeyaml to parse those files. That's why those findings are probably still treated as valid.

  1. There is nothing to fix in SnakeYAML. Otherwise please show what exactly.

This was a prediction on the known findings at that point.

  1. You cannot simple "use another tool" as you say. All the parsers have to follow the specification and it requires to support data structues which can be misused by a potential attacker.

I have to clarify this point perhaps: If hypthetically someone was coding a service, that uses yaml formatted user input, there would be the freedom to use any yaml parser. Using snakeyaml 1.30 would be not the best idea, because of the DoS risks.

  1. Please calify this statement "I expect some updates of snakeyaml in the near future." - what exactly you expect ?

See again the vulnerability count on mvnrepository.com Snakeyaml project apparently got ahead of the race and managed to close most of the findings, that can be closed in an API compatible way (without major version update). @wilkinsona : Perhaps it is time to think about updating to snakeyaml 1.33? It has 6 fewer vulerabilities, which are transitively also reflecting on sping-boot project.

asomov commented 1 year ago

@robert-gdv

I constantly receive the message that it is complex to check the vulnerability properly. I find it no excuse to substitute low quality with inability. Image you do it for anything else (it is difficult to create concurrent applications, so we just let the application to have low quality and fail in production). I am told "this is the industry state, we do not have anything better". At least everybody should be aware of the very low state to properly ignore the invalid warnings.

kendarkfire commented 1 year ago

Spring Boot 3.0.5 upgraded it to SnakeYAML 1.33, but there is still a critical VULNERABILITIES in SankeYAML 1.33: CVE-2022-1471, it should upgrade it to SankeYAML 2.0

asomov commented 1 year ago

@kendarkfire it is a false positive (as many many others). Spring Boot uses SafeConsturctor. Please read the CVE-2022-1471

kendarkfire commented 1 year ago

@asomov Yes, the spring boot could be safe, but the spring boot users may use the unsafe SankeYAML constructor in their own code if they don't know the VULNERABILITIES CVE-2022-1471

asomov commented 1 year ago

@kendarkfire what it has to do with Spring Boot ? Why do you report it here ? They can make a mistake with or without Spring. Please do not spread the false positive.

rivancic commented 1 year ago

Thanks to both @kendarkfire and especially @asomov 💯 for sharing information here. It was helpful for me to grasp faster what is going on with this reported CVE. @asomov I agree with you at the core it isn't Spring teams fault, but (unfortunately) not everyone is an expert at this topics, at least teams are drawn attention with the CVE and then it can be properly researched and act accordingly (Make sure you don't use unsafe constructor if parsing untrusted yaml files with snake.yaml library).

I'm referencing following issue: https://github.com/spring-projects/spring-boot/issues/35064. As I understand, if you are in a panic mode you can override the version of snake yaml to version 2.0 without breaking changes for your standard application configuration file (can't confirm didn't tried it yet) Explanation in SO -> https://stackoverflow.com/questions/75870282/is-snakeyaml-2-0-added-in-the-new-spring-boot-versions

asomov commented 1 year ago

@rivancic I am sorry, I did not get you. Especially the "panic mode".

rivancic commented 1 year ago

"panic mode" - you want ASAP get rid of the CVE warning from any of the dependency scanning tools.

bclozel commented 1 year ago

@rivancic If you're in "panic mode" about this particular CVE, you're 8 months late as we've released Spring Boot versions that are compatible with 1.31+ for a while now. You can run SnakeYaml 2.0 with compatible Spring Boot versions.

asomov commented 1 year ago

@rivancic the best thing you can do: