snakeyaml cve fix #24099

nishithakbhaskaran · 2024-11-20T09:04:37Z

Description

Upgrade snakeyaml version to 2.0
Presto Cassandra Tests were failing because the cassandra-server version 2.1.16-1 using sankeyaml 1.x as tranistiive dependency and if we update the version it will cause some methodNotFound Error. Inorder to overcome this we removed the cassandra-server from presto and cherrypicked the dockerised cassandra in tests as done in Use dockerized Cassandra in tests trinodb/trino#2377 . This approach will resolve the issue.

Motivation and Context

snakeyaml 1.x version which is coming as a transitive dependency introduces below High and Medium CVEs in Mend Scan.

CVE-2022-1471
CVE-2022-25857
CVE-2017-18640
CVE-2022-38752
CVE-2022-38751
CVE-2022-38750
CVE-2022-38749
CVE-2022-41854

Presto Cassandra Tests were failing because the cassandra-server version 2.1.16-1 using sankeyaml 1.x as tranistiive dependency and if we update the version it will cause some methodNotFound Error. Inorder to overcome this we removed the cassandra-server from presto and cherrypicked the dockerised cassandra in tests as done in Use dockerized Cassandra in tests trinodb/trino#2377 . This approach will resolve the issue.

Impact

Test Plan

Contributor checklist

Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
Documented new properties (with its default value), SQL syntax, functions, or other functionality.
If release notes are required, they follow the release notes guidelines.
Adequate tests were added if applicable.
CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

Security Fixes
* Upgrade snakeyaml to 2.0 in response to `CVE-2022-1471 <https://nvd.nist.gov/vuln/detail/CVE-2022-1471>`_. :pr:`24099`
* Upgrade snakeyaml to 2.0 in response to `CVE-2022-25857 <https://nvd.nist.gov/vuln/detail/cve-2022-25857>`_. :pr:`24099`
* Upgrade snakeyaml to 2.0 in response to `CVE-2017-18640 <https://nvd.nist.gov/vuln/detail/cve-2017-18640>`_. :pr:`24099`
* Upgrade snakeyaml to 2.0 in response to `CVE-2022-38752 <https://nvd.nist.gov/vuln/detail/CVE-2022-38752>`_. :pr:`24099`
* Upgrade snakeyaml to 2.0 in response to `CVE-2022-38751 <https://nvd.nist.gov/vuln/detail/CVE-2022-38751>`_. :pr:`24099`
* Upgrade snakeyaml to 2.0 in response to `CVE-2022-38750 <https://nvd.nist.gov/vuln/detail/CVE-2022-38750>`_. :pr:`24099`
* Upgrade snakeyaml to 2.0 in response to `CVE-2022-38749 <https://nvd.nist.gov/vuln/detail/CVE-2022-38749>`_. :pr:`24099`
* Upgrade snakeyaml to 2.0 in response to `CVE-2022-41854 <https://nvd.nist.gov/vuln/detail/CVE-2022-41854>`_. :pr:`24099`

nishithakbhaskaran · 2024-11-25T09:51:19Z

Presto Cassandra Tests were failing because the cassandra-server version 2.1.16-1 using sankeyaml 1.x as tranistiive dependency and if we update the version it will cause some methodNotFound Error. Inorder to overcome this we removed the cassandra-server from presto and cherrypicked the dockerised cassandra in tests as done in Trino . This approach will resolve the issue.

presto-cassandra/src/test/java/com/facebook/presto/cassandra/CassandraQueryRunner.java

yingsu00 · 2024-11-26T08:04:58Z

presto-cassandra/src/test/java/com/facebook/presto/cassandra/TestCassandraConnector.java

@@ -94,6 +93,8 @@ public class TestCassandraConnector
            ImmutableSet.of(),
            Optional.empty(),
            ImmutableMap.of());
+
+    private CassandraServer server;


Move this private member to the private section below.

yingsu00 · 2024-11-26T08:11:29Z

presto-cassandra/src/test/resources/cu-cassandra.yaml

@@ -94,10 +94,10 @@ partitioner: org.apache.cassandra.dht.Murmur3Partitioner

 # directories where Cassandra should store data on disk.
 data_file_directories:
-    - ${data_directory}/data
+    - /var/lib/cassandra/data


What would happen if /var/lib/cassandra does not exist?

This is the path in Cassandra container and when we start the Cassandra by default will write its data files there.

yingsu00 · 2024-11-26T08:12:50Z

presto-cassandra/src/test/java/com/facebook/presto/cassandra/CassandraServer.java

@@ -123,27 +114,19 @@ private static String prepareCassandraYaml()
        return yamlLocation.toAbsolutePath().toString();
    }

-    public static synchronized CassandraSession getSession()
+    public CassandraSession getSession()


For line 109:

Since you removed data_directory from cu-cassandra.yaml, this is no longer needed. However we need to make sure the path you specified always exist and won't cause error on different platforms.

Sure I have removed the line. Also the path is inside cassandra docker container and it will be independent of the platforms, so I believe this won't cause issues. I have updated the cherrypick id in the description of the PR.

yingsu00 · 2024-11-26T08:19:42Z

presto-cassandra/pom.xml

+    <dependencyManagement>
+        <dependencies>
+            <dependency>
+                <groupId>org.slf4j</groupId>


Where is this bring used?

This dependencyManagement section is to fix the Require upper bound dependencies error for slf4j while building the cassandra module.

Can you explain a little bit about why the error happens if this one was not added? Is org.testcontainers dependent on slf4j or something else?

Yes @yingsu00 .

slf4j comes as a transitive dependency in org.testcontainers. If we do not add this dependencymanagement tag
the following error will come.

Rule 0: org.apache.maven.plugins.enforcer.RequireUpperBoundDeps failed with message: Failed while enforcing RequireUpperBoundDeps. ..... ..... [ERROR] Failed to execute goal org.apache.maven.plugins:maven-enforcer-plugin:3.0.0-M2:enforce (default) on project presto-cassandra: Some Enforcer rules have failed. Look above for specific messages explaining why the rule failed.

Inorder to fix this I have added the slf4j version as dependencyManagement tag.

Yes @yingsu00 .

slf4j comes as a transitive dependency in org.testcontainers. If we do not add this dependencymanagement tag the following error will come.

Rule 0: org.apache.maven.plugins.enforcer.RequireUpperBoundDeps failed with message: Failed while enforcing RequireUpperBoundDeps. ..... ..... [ERROR] Failed to execute goal org.apache.maven.plugins:maven-enforcer-plugin:3.0.0-M2:enforce (default) on project presto-cassandra: Some Enforcer rules have failed. Look above for specific messages explaining why the rule failed.

Inorder to fix this I have added the slf4j version as dependencyManagement tag.

If that's the case, can you please add a comment in the code?

@yingsu00 Added the comment.

yingsu00

In the commit message, there should be an empty line after the commit tile.

nishithakbhaskaran · 2024-11-28T06:11:41Z

In the commit message, there should be an empty line after the commit tile.

@yingsu00 Updated the commit message.Please verify

added comment in pom

prestodb-ci added the from:IBM PR from IBM label Nov 20, 2024

prestodb-ci requested a review from a team November 20, 2024 09:04

nishithakbhaskaran force-pushed the snakeyaml branch 13 times, most recently from 522f10d to 42b67e8 Compare November 22, 2024 13:36

nishithakbhaskaran marked this pull request as ready for review November 22, 2024 14:40

nishithakbhaskaran requested a review from a team as a code owner November 22, 2024 14:40

nishithakbhaskaran requested a review from presto-oss November 22, 2024 14:40

ethanyzhang requested review from a team, psnv03 and pramodsatya and removed request for a team November 25, 2024 07:04

nishithakbhaskaran marked this pull request as draft November 25, 2024 07:05

nishithakbhaskaran marked this pull request as ready for review November 25, 2024 09:29

yingsu00 reviewed Nov 26, 2024

View reviewed changes

nishithakbhaskaran force-pushed the snakeyaml branch 3 times, most recently from 229bdf6 to 0842929 Compare November 26, 2024 10:36

yingsu00 reviewed Nov 28, 2024

View reviewed changes

nishithakbhaskaran force-pushed the snakeyaml branch 8 times, most recently from e7f0e93 to 568312e Compare November 28, 2024 06:10

nishithakbhaskaran force-pushed the snakeyaml branch 3 times, most recently from ae78f02 to 1d697f8 Compare December 3, 2024 04:41

Fix snakeyaml CVEs

2abda22

added comment in pom

nishithakbhaskaran force-pushed the snakeyaml branch from 1d697f8 to 2abda22 Compare December 3, 2024 05:01

yingsu00 approved these changes Dec 3, 2024

View reviewed changes

yingsu00 merged commit 9eeac24 into prestodb:master Dec 3, 2024
57 checks passed

nishithakbhaskaran deleted the snakeyaml branch December 11, 2024 07:05

This was referenced Jan 28, 2025

Add release notes for 0.291 #24445

Merged

Add release notes for 0.291 #24448

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

snakeyaml cve fix #24099

snakeyaml cve fix #24099

nishithakbhaskaran commented Nov 20, 2024 •

edited

Loading

nishithakbhaskaran commented Nov 25, 2024

yingsu00 Nov 26, 2024

nishithakbhaskaran Nov 26, 2024

yingsu00 Nov 26, 2024

nishithakbhaskaran Nov 26, 2024

yingsu00 Nov 26, 2024

nishithakbhaskaran Nov 26, 2024

yingsu00 Nov 26, 2024

nishithakbhaskaran Nov 26, 2024

yingsu00 Nov 28, 2024

nishithakbhaskaran Nov 28, 2024 •

edited

Loading

yingsu00 Dec 2, 2024

nishithakbhaskaran Dec 3, 2024

yingsu00 left a comment

nishithakbhaskaran commented Nov 28, 2024

snakeyaml cve fix #24099

snakeyaml cve fix #24099

Conversation

nishithakbhaskaran commented Nov 20, 2024 • edited Loading

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

Release Notes

nishithakbhaskaran commented Nov 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nishithakbhaskaran Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yingsu00 left a comment

Choose a reason for hiding this comment

nishithakbhaskaran commented Nov 28, 2024

nishithakbhaskaran commented Nov 20, 2024 •

edited

Loading

nishithakbhaskaran Nov 28, 2024 •

edited

Loading