Bump lz4-java to 1.10.2 for CVE-2025-12183 & CVE-2025-66566 fixes.#14941
Bump lz4-java to 1.10.2 for CVE-2025-12183 & CVE-2025-66566 fixes.#14941slfan1989 wants to merge 1 commit intoapache:mainfrom
Conversation
04cf844 to
3906ecc
Compare
3906ecc to
805cd56
Compare
|
@huaxingao Could you please help review this PR? Thank you very much! |
|
I’m seeing org.lz4:lz4-java:1.8.0 still present on Spark 3.5/4.0 compileClasspath. Does this need to be fixed too? |
Thanks for the review! You're right — even as a transitive dependency, the vulnerable I'll update the PR to add a global Pushing the change shortly. Thanks again! |
build.gradle
Outdated
| project.name.startsWith('iceberg-kafka-connect')) { | ||
|
|
||
| configurations.all { | ||
| exclude group: 'org.lz4', module: 'lz4-java' |
There was a problem hiding this comment.
This is a global exclusion rule. After applying this rule, we can use the following command for validation.
gradlew --parallel \
-DsparkVersions=4.0 \
-DscalaVersion=2.13 \
:iceberg-spark:iceberg-spark-4.0_2.13:dependencies \
:iceberg-spark:iceberg-spark-extensions-4.0_2.13:dependencies \
:iceberg-spark:iceberg-spark-runtime-4.0_2.13:dependencies \
--configuration compileClasspath \
-Pquick=true \
--refresh-dependencies|grep "lz4"
The result is as follows:
| +--- at.yawk.lz4:lz4-java:1.10.2
| +--- at.yawk.lz4:lz4-java:1.10.2
| +--- at.yawk.lz4:lz4-java:1.10.2
| +--- at.yawk.lz4:lz4-java:1.10.2
| +--- at.yawk.lz4:lz4-java:1.10.2
|
@huaxingao Could you please help review this PR again? Thank you very much! |
build.gradle
Outdated
| } | ||
| } | ||
|
|
||
| configurations.all { |
There was a problem hiding this comment.
Is this change still needed?
There was a problem hiding this comment.
Thanks for the review and suggestions! I agree—this part can be removed. I'll make further improvements and update the PR soon.
build.gradle
Outdated
| if (project.name.startsWith('iceberg-spark') || | ||
| project.name.startsWith('iceberg-flink') || | ||
| project.name.startsWith('iceberg-delta-lake') || | ||
| project.name.startsWith('iceberg-kafka-connect')) { |
There was a problem hiding this comment.
why only these module, shouldn't we doing it for all ?
There was a problem hiding this comment.
Currently, only a few modules depend on org.lz4:lz4-java. Promoting this dependency to the project level is safe and will improve consistency and standardization in dependency management. I plan to implement this change.
There was a problem hiding this comment.
Is it better to keep this rule scoped (e.g., Spark/Flink/Kafka Connect) because the vulnerable org.lz4:lz4-java is coming from the Spark/Flink/Kafka dependency trees?
There was a problem hiding this comment.
@huaxingao Thank you very much for reviewing the code and for the helpful suggestions.
From my perspective, I also lean toward scoping this rule to the relevant components (e.g., Spark / Flink / Kafka Connect), since the current org.lz4:lz4-java vulnerability is primarily introduced via transitive dependencies in the Spark/Flink/Kafka dependency trees. This would help reduce the impact on other unrelated modules.
From my side: +1 to scoping the rule to Spark / Flink / Kafka Connect.
@singhpk234 Can you agree with this improvement?
There was a problem hiding this comment.
@huaxingao @singhpk234 Could you please take another look at this PR? Thank you very much!
There was a problem hiding this comment.
I agree with @singhpk234 here and I think we should override the dependency for all iceberg modules. That way we can't accidentally introduce it anywhere else. Also the changes in L372 for the iceberg-core module won't be necessary
9a950df to
7a7eee9
Compare
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
|
@huaxingao @singhpk234 @nastra Could you please take another look at this PR? Thank you very much! |
build.gradle
Outdated
| javaPlatform { allowDependencies() } | ||
| } | ||
|
|
||
| subprojects { |
There was a problem hiding this comment.
we already have a subprojects section in L124, so better to move this stuff there
There was a problem hiding this comment.
Makes sense. We already have the Subprojects section at L124, so I’ll move this content there.
build.gradle
Outdated
| configurations.all { | ||
| exclude group: 'org.lz4', module: 'lz4-java' | ||
| resolutionStrategy.capabilitiesResolution.withCapability("org.lz4:lz4-java") { | ||
| select("at.yawk.lz4:lz4-java:0") |
There was a problem hiding this comment.
what does the :0 do there?
select("at.yawk.lz4:lz4-java:0") tells Gradle to resolve org.lz4:lz4-java capability conflicts by choosing the at.yawk.lz4 variant, while keeping the version decided by the normal dependency graph (1.10.2 here). The :0 is just a required placeholder — it has no real effect on the version chosen.
build.gradle
Outdated
| exclude group: 'org.lz4', module: 'lz4-java' | ||
| resolutionStrategy.capabilitiesResolution.withCapability("org.lz4:lz4-java") { | ||
| select("at.yawk.lz4:lz4-java:0") | ||
| because("Fix lz4-java capability conflict from relocation and CVE fixes") |
There was a problem hiding this comment.
not sure I understand what "from relocation" refers to. Maybe this should say Enforce lz4-java that contains CVE-X and CVE-Y fixes
There was a problem hiding this comment.
Thanks for your suggestions—I’ll work on improving this part.
25a4b52 to
2f4d3a3
Compare
Why are the changes needed?
To fully address two recently disclosed CVEs in lz4-java:
The current version in main (1.10.2) already includes the fix from 1.10.1 and subsequent improvements. This PR ensures we are on the latest patch release to eliminate any vulnerability scanner alerts and benefit from minor bug fixes/performance improvements.
References: