Build: bump Jackson to fix CVE GHSA-r7wm-3cxj-wff9 - #17336
Conversation
|
@Guosmilesmile thanks for putting this fix. Seeing CI failures in other PRs. Would be great to get this in. Do you mind adding the description to the PR so the change rationale is clear? |
OK |
|
Thanks @Guosmilesmile! Please rebase above the current as I like your better. Maybe we can even try to upgrade Spark 3.5 to the new version too. If that works that would be even better. |
@pvary Thank you very much for the reminder. I removed the "ignore" section for Spark, and aligned the 3.5 version with the 4.1 version by upgrading it. |
| force "com.fasterxml.jackson.module:jackson-module-scala_${scalaVersion}:${libs.versions.jackson221.get()}" | ||
| force "com.fasterxml.jackson.core:jackson-databind:${libs.versions.jackson221.get()}" | ||
| force "com.fasterxml.jackson.core:jackson-core:${libs.versions.jackson221.get()}" |
There was a problem hiding this comment.
If we do this, do we need the GHSA-r7wm-3cxj-wff9 in spark-runtime-3.5.trivyignore?
There was a problem hiding this comment.
Also, how sure are we that Spark works with a different fasterxml version?
There was a problem hiding this comment.
If we do this, do we need the GHSA-r7wm-3cxj-wff9 in spark-runtime-3.5.trivyignore?
Yes, so I remove GHSA-r7wm-3cxj-wff9 in spark-runtime-3.5.trivyignore.
Also, how sure are we that Spark works with a different fasterxml version?
Good question. Since com.fasterxml is relocated to org.apache.iceberg.shaded.com.fasterxml(relocate 'com.fasterxml', 'org.apache.iceberg.shaded.com.fasterxml'), the Jackson copy included in the Iceberg runtime jar is isolated from Spark's own Jackson jars and should not override them.
I verified dependency resolution for Spark 3.5 and it now resolves jackson-core, jackson-databind, and jackson-module-scala_2.12 to 2.21.4.
However, that is not a full compatibility guarantee, so we should validate this with Spark 3.5 tests.
There was a problem hiding this comment.
Good. Let's remove and see.
Also we might want to ask someone with more spark experience.
Maybe @szehon-ho or @huaxingao?
What do you think @szehon-ho or @huaxingao: If we change the relocated jackson version from 2.15.2 to 2.21.4, would that cause any issues for the users if the Spark test are green?
There was a problem hiding this comment.
My previous submission has already removed the GHSA-r7wm-3cxj-wff9 in spark-runtime-3.5.trivyignore.
As for the rest, let's hear what others think.
There was a problem hiding this comment.
each version of spark is using a different override right now
- spark v3.5 is currently using jackson215
- spark v4.0 is currently using jackson218
- spark v4.1 is currently using jackson221
Looks like the CVE suggests 2.18.8 as the minimum version, GHSA-r7wm-3cxj-wff9
Should we use that instead of 2.21?
There was a problem hiding this comment.
Do we actually need this bump? The CVE (GHSA-r7wm-3cxj-wff9) is a DoS in Jackson's async/non-blocking parser, which isn't used on Iceberg's parsing paths (we use the blocking parser), and the jar is shaded — so it's not a reachable issue for us, just a scanner finding we already suppress.
If we do want to fix it (e.g. to keep downstream scanners quiet), I'd lean toward 2.18.8: it's the minimal patched version that clears the finding (Spark 4.0 ships it and needs no ignore), and it's the smallest jump from 3.5's native 2.15.2.
There was a problem hiding this comment.
Do we actually need this bump?
I agree. If we dont exercise this particular code path then lets not bump this and avoid potentially introducing new issues
There was a problem hiding this comment.
This PR was originally meant to fix the CVEs for both Kafka and Spark together. The Kafka part has already been merged. As for the Spark part, I feel that if we can fix the vulnerability by bumping the jar version, that might be a cleaner approach than just suppressing it in spark-runtime-3.5.trivyignore.
That said, I'm happy to go with whatever the majority thinks is best here :)
|
Change spark jackson 2.21 to 2.18 to keep the smallest jump |
| # jackson-databind CVEs that are only fixed in jackson >= 2.18.8. | ||
| CVE-2026-54512 | ||
| CVE-2026-54513 | ||
| GHSA-r7wm-3cxj-wff9 |
There was a problem hiding this comment.
is this file still accurate after bumping jackson to 2.18.8?
There was a problem hiding this comment.
Thanks for pointing it out. Also remove CVE-2026-54512 and CVE-2026-54513.
There was a problem hiding this comment.
looks like this file is now empty, and we can either remove it completely or keep only a skeleton. but we should remove the lines about jackson
There was a problem hiding this comment.
Make sense. Remove the lines about jackson now.
kevinjqliu
left a comment
There was a problem hiding this comment.
LGTM.
I see the tradeoff as:
- Upgrade to 2.18.8: Small compatibility risk, mitigated by shading and the existing test coverage.
- Stay on 2.15: Very low current exploitability, but continue shipping known-vulnerable code with future reachability and compliance risk.
I lean towards the upgrade. The current tests provide sufficient coverage to keep the compatibility risk low.
| # jackson-databind CVEs that are only fixed in jackson >= 2.18.8. | ||
| CVE-2026-54512 | ||
| CVE-2026-54513 | ||
| GHSA-r7wm-3cxj-wff9 |
There was a problem hiding this comment.
looks like this file is now empty, and we can either remove it completely or keep only a skeleton. but we should remove the lines about jackson
mbutrovich
left a comment
There was a problem hiding this comment.
I lean towards the upgrade. The current tests provide sufficient coverage to keep the compatibility risk low.
I agree with this assessment. I think the reasoning to do a minimal 2.18.8 bump, despite Iceberg not using the code path in question, is the right call. We're minimizing the risk of surprise changes by not jumping to a much newer version, and we have CI as a backstop for correctness.
| CVE-2026-54512 | ||
| CVE-2026-54513 | ||
| # jackson-core CVE shaded into parquet-jackson, only fixed in jackson | ||
| # >= 2.18.8 / 2.21.4 / 2.22.1. | ||
| GHSA-r7wm-3cxj-wff9 |
There was a problem hiding this comment.
we should look into cleaning these up too
There was a problem hiding this comment.
@Guosmilesmile do you mind following up with the items here?
There was a problem hiding this comment.
I will raise a pr to fix it later
|
Im comfortable moving forward with this PR, thanks @Guosmilesmile for the fix and everyone for the review! |
This PR updates the Spark 3.5 runtime Jackson dependency alignment to address
GHSA-r7wm-3cxj-wff9.jackson215version set tojackson218.jackson-coreandjackson-databind.parquet-jackson, which cannot be upgraded independently of Apache Parquet.