testcontainers-java icon indicating copy to clipboard operation
testcontainers-java copied to clipboard

[Bug]: Testcontainers uses outdated versions of Snakeyaml & Jackson, which have critical vulnerabilities, flagging testcontainers on security scanning tools

Open ZachChuba opened this issue 1 year ago • 23 comments

Module

Core

Testcontainers version

1.20.1

Using the latest Testcontainers version?

Yes

Host OS

MacOS

Host Arch

ARM

Docker version

Client:
 Version:           26.1.4
 API version:       1.45
 Go version:        go1.21.11
 Git commit:        5650f9b
 Built:             Wed Jun  5 11:26:02 2024
 OS/Arch:           darwin/arm64
 Context:           desktop-linux

What happened?

The testcontainers core shades snakeyaml 1.33 into the jar. Snakeyaml 1.33 is vulnerable to CVE-2022-1471. Even though the code does not appear vulnerable to this issue because it uses SafeConstructor, enterprises may blacklist testcontainers for the mere presence of snakeyaml 1.33. Please consider upgrading to snakeyaml 2.0 or higher.

Relevant log output

No response

Additional Information

https://nvd.nist.gov/vuln/detail/CVE-2022-1471

ZachChuba avatar Sep 30 '24 19:09 ZachChuba

I can see that the latest versions of snakeyaml do not have any vulnerabilities(https://mvnrepository.com/artifact/org.yaml/snakeyaml). Is upgrading to the latest version the only expectation? Then, I can create a pull request to upgrade it.

mranjit avatar Oct 04 '24 15:10 mranjit

Yes version 2.0 and above remediate it, upgrading to 2.2 would work

ZachChuba avatar Oct 04 '24 16:10 ZachChuba

Please, do not open a PR for this. We should take into account when update a dependency to a major version when doing patch and minor releases.

eddumelendez avatar Oct 04 '24 16:10 eddumelendez

Any updates on the status of this vulnerability? It is causing some concerns for us.

cc @eddumelendez

PiotrSierkin-Ki avatar Oct 16 '24 11:10 PiotrSierkin-Ki

Also for us, Nexus IQ/Sonatype is blocking this

Orbifoldt avatar Oct 17 '24 14:10 Orbifoldt

CVE-2022-1471 SnakeYaml's Constructor() class does not restrict types which can be instantiated during deserialization. Deserializing yaml content provided by an attacker can lead to remote code execution. We recommend using SnakeYaml's SafeConsturctor when parsing untrusted content to restrict deserialization. We recommend upgrading to version 2.0 and beyond. testcontainers-1.20.5.jar: org/testcontainers/shaded/org/yaml/snakeyaml/Yaml.class(, 2.0)

gquintana avatar Feb 21 '25 07:02 gquintana

This is yet another false positive. The issue is very explicit about the untrusted source. Testcontainers use YAML as configuration from the classpath (not downloading it from some external URL without any control for the YAML provider). Important: the CVE is correct, it warns and explains the context, but the tool which creates the false positive should be amended to radically improve the quality and stop sending false information blocking the developers. Since the code uses SafeConstructor, there is no issue at all. So it is double false positive. By the way, I can help to migrate to version 2 of SnakeYAML. The API is not backward compatible with version 1

Disclosure: in version 2, SnakeYAML's Constructor() class ALSO does not resctrict instantiation of arbitrary classes, but the tooling does not complain :-/

asomov avatar Feb 27 '25 10:02 asomov

I have started the migration, but the tests in the main branch failed:

* What went wrong:
Execution failed for task ':activemq:spotlessJava'.

I do not want to make a PR without all the tests green. Can you please help me?

asomov avatar Feb 27 '25 16:02 asomov

Don't forget the other false positive -- test containers is only properly used in testing, so even if it were exploitable it would have trivial impact. Unfortunately, though, tools like sonatype only have the intelligence to assess presence of a vulnerable version, not if it is exploitable, or even used in the code. It's doubly annoying when a large enterprise has risk managers who have 0 care about exploitability and just want green charts, but have the power to block releases or even the dependency existing in the company's ecosystem because the chart isn't green. However, that's something many developers at enterprises deal with daily :(

ZachChuba avatar Feb 28 '25 01:02 ZachChuba

@ZachChuba I wonder why the developers tolerate these low quality tooling which create a stream of false positives. @gquintana since testcontainers already uses SafeConstructor, why should the project migrate? What is the added value? (except to satisfy the false positive generators)

asomov avatar Feb 28 '25 07:02 asomov

@asomov I truly understand your point.

Sadly, being considered as dangerous, nearly all testcontainers releases are considered as "security critical" and quarantined by Nexus dependency proxy. As a result, we can not use it at all, except very old releases.

I wonder whether having a testcontainer artefact without any dependency shaded in it (No Jackson, no SnakeYaml) could be a workaround. In our case most SnakeYaml releases are considered as "security critical" but very few are quarantined (don't ask me why). Same for Jackson, some releases are marked as "security critical" but none of them is quarantined. In the end the best solution could be to have a "slim" testcontainers Jar and let users manage their dependency hell by their own.

gquintana avatar Mar 01 '25 07:03 gquintana

@gquintana what is "Nexus dependency proxy"? Is is your corporate component? This is nice your understand the point. In this case please do not ask to "improve or fix" testcontainers. They do things properly! Please go to the low quality tooling which generates the false positives and create bug reports there. This is important.

asomov avatar Mar 01 '25 08:03 asomov

@asomov I perfectly know the value of testcontainers, this why I am trying to find a solution to be able to use it.

I won't argue about the quality of the Sonatype tool used to prevent developers from downloading "unsecure" dependencies. With cyber security related topics it's usually very hard to argue, most of the time we can just deal with it. The Log4Shell issue brought dependencies management under the light.

gquintana avatar Mar 01 '25 09:03 gquintana

@gquintana well, if the quality of the tooling is known to be low, then the complain should be somewhat different. @PiotrSierkin-Ki is it still an issue? Have you filed a bug to the issue tracker of the tool which generates the false positive?

asomov avatar Mar 01 '25 09:03 asomov

Sonatype is a useful tool for identifying the potentiality of vulnerabilities in a project, but it is horrible because enterprises typically equate the potentiality of a vulnerability with the exploitability of one. And even if proven non-exploitable sonatype might still not allow it in the software ecosystem.

Yes, we developers have complaints about the tooling, but the tooling doesn't claim to assess exploitability. It essentially just scans the dependencies list and shaded classpath, takes a hash of every file and searches their database to see if that version of the file is associated with a CVE or sonatype cataloged exploit and then generates a report based on the presence. Sonatype will never mark this as a false positive because a vulnerable class file is in the classpath which is the extent of their guarantees - regardless of if that class file is actually used anywhere.

As @gquintana said, this is usually pretty easy to "just deal with" instead of arguing when the dependencies are not shaded in. Problem is when it's shaded in and there's dependency clashes that make upgrades more difficult and require changing source code. I have forked testcontainers and created a version without these sonatype complaints which serves my specific purposes for the containers my team uses, but it does not pass tests for some containers so I'm not going to be able to raise a PR for it.

ZachChuba avatar Mar 02 '25 01:03 ZachChuba

@ZachChuba for your information - they do not scan any hashes, they simply check the version. Very primitive. This issue should not have been created, and the community should not waste tons of time and energy to resolve a false positive. The issue should not be fixed here but in the "scanner" which generates a false positive. Let us fix the cause, not the consequence.

asomov avatar Mar 02 '25 10:03 asomov

@ZachChuba can you please amend the title of this issue? It is confusing, it looks like a bug and it mentions Vulnerable Dependency Something like: escape a false positive for properly implementred SnakeYAML usage to avoid a build block

asomov avatar Mar 03 '25 06:03 asomov

@ZachChuba I wonder why the developers tolerate these low quality tooling which create a stream of false positives.

Tools like Nexus IQ/Sonatype which are blocking it aren't loved by developers in corporates but are a necessary evil due to large number of CVEs in production code through dependencies. Test specific code like this then gets caught crossfire off it.

@gquintana since testcontainers already uses SafeConstructor, why should the project migrate? What is the added value? (except to satisfy the false positive generators)

At some point the project will migrate anyway, you won't want to always stay on the old version. The question then becomes when should be do it? The advantage for the community is hopefully that some of these corporate users will contribute back.

@asomov I have started the migration, but the tests in the main branch failed:

I can spare some time and can help if needed, assuming it'll get merged, I saw @eddumelendez saying not to open a PR some time back when I first looked at this issue so didn't spend any time working on it

codefish1 avatar May 06 '25 14:05 codefish1

@codefish1 I like your wisdom.

Question then becomes when should be do it? I would say, once in a while (the actual SnakeYAML 1.33 released in 2022 is 3 years old), when there are enough reasons to upgrade (CVEs being one of them to me), and when many users are complaining (I mean reporting the issue, struggling to workaround it or to fix it).

gquintana avatar May 07 '25 05:05 gquintana

@asomov I have started the migration, but the tests in the main branch failed:

I can spare some time and can help if needed, assuming it'll get merged, I saw @eddumelendez saying not to open a PR some time back when I first looked at this issue so didn't spend any time working on it

@ZachChuba I did not quite catch you. There is nothing to merge. The lastest main branch fails:

`commit e730674537dfa297a60311a7c13b953b8b82a67d (HEAD -> main, origin/main, origin/HEAD) Author: Eddú Meléndez Gonzales [email protected] Date: Fri Apr 11 17:11:50 2025 -0600

Allow spock tests to be skipped when Docker is unavailable (#10180)

Fixes #10178

`

Image

asomov avatar May 07 '25 07:05 asomov

Is there a reason the dependencies are shaded in? That's what causes the problem for us corporate users, since we can't override them with allowable versions

codefish1 avatar May 07 '25 21:05 codefish1

I am working on a PR to upgrade snakeyaml and jackson to the latest versions. @codefish1 the dependencies were previously shaded in to hard enforce backwards compatibility for certain containers, specifically k3s.

ZachChuba avatar May 08 '25 17:05 ZachChuba

@codefish1 the dependencies were previously shaded in to hard enforce backwards compatibility for certain containers, specifically k3s.

Interesting. I've not seen those problems with transitive dependencies before. I guess it's caused by the apps using newer versions of these dependencies which potentially cause problems.

codefish1 avatar May 09 '25 06:05 codefish1