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

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

Open
ZachChuba opened this issue Sep 30, 2024 · 17 comments
Labels

Comments

@ZachChuba
Copy link

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

@mranjit
Copy link

mranjit commented Oct 4, 2024

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.

@ZachChuba
Copy link
Author

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

@eddumelendez
Copy link
Member

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.

@PiotrSierkin-Ki
Copy link

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

cc @eddumelendez

@Orbifoldt
Copy link

Also for us, Nexus IQ/Sonatype is blocking this

@gquintana
Copy link

gquintana commented Feb 21, 2025

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)

@asomov
Copy link

asomov commented Feb 27, 2025

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
Copy link

asomov commented Feb 27, 2025

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?

@ZachChuba
Copy link
Author

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 :(

@asomov
Copy link

asomov commented Feb 28, 2025

@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)

@gquintana
Copy link

gquintana commented Mar 1, 2025

@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.

@asomov
Copy link

asomov commented Mar 1, 2025

@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.

@gquintana
Copy link

gquintana commented Mar 1, 2025

@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.

@asomov
Copy link

asomov commented Mar 1, 2025

@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?

@ZachChuba
Copy link
Author

ZachChuba commented Mar 2, 2025

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.

@asomov
Copy link

asomov commented Mar 2, 2025

@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
Copy link

asomov commented Mar 3, 2025

@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

@ZachChuba ZachChuba changed the title [Bug]: Vulnerable Dependency CVE NVD 9.8 Snakeyaml [Bug]: Testcontainers uses outdated versions of Snakeyaml & Jackson, which have critical vulnerabilities, flagging testcontainers on security scanning tools Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants