Repository navigation
Conversation
|
The job above didn't scan all modules. You can refer to the job in this following link to find out all reported CVEs to check whether we should suppress these CVEs https://github.com/apache/druid/actions/runs/33229536711/job/99039733975?pr=20126 |
Druid's web console only calls uuidv4() with no buf argument, so the vulnerable v3/v5/v6 code path is never exercised. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 1 |
| P3 | 0 |
| Total | 2 |
Reviewed 1 of 1 changed files.
This is an automated review by Codex GPT-5.6-Luna(max)
| <cve>CVE-2026-45674</cve> <!-- Affects netty-resolver-dns which Druid doesn't use; Druid uses JDK DNS resolution --> | ||
| <cve>CVE-2026-48006</cve> <!-- We don't use the Redis codec --> | ||
| <cve>CVE-2026-42585</cve> <!-- Server-side HTTP request smuggling; Druid's HTTP server is Jetty, not Netty --> | ||
| <cve>CVE-2026-42584</cve> <!-- HttpClientCodec response desynchronization in Netty 4.x codec; Druid uses Netty 3.x's HttpClientCodec which has a different implementation --> |
There was a problem hiding this comment.
[P1] Do not suppress the Netty response desynchronization
The new suppression is unsafe for the Netty 3 client used by Druid. Netty 3's HttpClientCodec queues request methods and only treats status 100 as an empty informational response; a 103 response can therefore complete the current response early while the connection remains reusable, desynchronizing the next request's response. Keep this CVE unsuppressed until the client holds the connection through the final response or the codec is patched or upgraded.
There was a problem hiding this comment.
Hmm, this does seem like a valid concern.
The CVE-2026-42584 does seem to affect all Netty versions lower than 4.1.133.
Let me double check on what can be done here, because upgrading Netty3 to Netty4 has already been discussed and there is also a PR (#19754 ) but we cannot block Druid 38 release on that.
There was a problem hiding this comment.
Thanks for checking. I re-verified this against the current head and the local netty-3.10.6.Final source. The suppression is still present at this line, but the comment's “Netty 4.x codec; different implementation” justification does not hold for the client Druid actually uses:
org.jboss.netty.handler.codec.http.HttpClientCodecskipsqueue.poll()only for status 100; status 103 falls through toqueue.poll(), while the base decoder still treats all 1xx responses as empty.NettyHttpClienttreats every non-chunked response as complete, removes its handler, and returns the channel toResourcePool. A server can therefore send an interim 103, cause the channel to be reused, and then deliver the original final response after the next request has been written.
That is the same unsafe response/request pairing described by CVE-2026-42584. Please remove this CVE from the Netty 3 suppression until this client waits for the final response or is patched/upgraded. If release timing requires accepting the exposure, it should be tracked as an explicit risk rather than suppressed as unaffected.
Reviewed 1 of 1 changed files.
There was a problem hiding this comment.
I checked this again, and it seems that the CVE affects 4.x versions only.
All sources agree that Netty3 is unaffected as it uses a different implementation.
There was a problem hiding this comment.
I re-checked the local netty-3.10.6.Final source and the Druid client lifecycle. The conclusion that Netty 3 is safe because it is a different implementation does not hold for this integration: org.jboss.netty.handler.codec.http.HttpClientCodec skips queue.poll() only for status 100; status 103 falls through to queue.poll(), and the base decoder treats every 1xx response as empty. Druid’s NettyHttpClient then calls finishRequest() for every non-chunked response and returns the channel to ResourcePool before the final response is consumed. A 103 followed by the final response can therefore reuse the socket with the previous response still in flight. Please remove CVE-2026-42584 from the Netty 3 suppression or explicitly track and mitigate the exposure until the client waits for final responses or is patched/upgraded.
Reviewed 1 of 1 changed files.
This is an automated review by Codex GPT-5.6-Luna(max)
There was a problem hiding this comment.
Thanks for the clarification. I rechecked the current head and agree that Druid does not intentionally pipeline requests, but the Netty 3/client lifecycle still creates the same overlap when a peer sends an interim 103:
- Netty 3
HttpClientCodecskips request-method dequeuing only for status 100; for 103 it polls the queued method, while the base decoder treats the 1xx response as content-empty. NettyHttpClientthen callsfinishRequest()for every non-chunked response and returns the channel toResourcePool. The next request can therefore be written on that channel while the prior request's final response is still outstanding: de facto pipelining caused by the decoder's premature completion.
The absence of HEAD requests does not remove this 103 path. Even for Druid-to-Druid traffic, a misbehaving or compromised peer can trigger it. Unless this is being accepted as an explicit trusted-peer risk, I recommend keeping CVE-2026-42584 unsuppressed until the client waits for the final response or is patched/upgraded.
Reviewed 1 of 1 changed files.
There was a problem hiding this comment.
IMO, we can accept the risk given that we're using this to contact other Druid servers in the same cluster. They should be able to trust each other. #19567 is making some more movement now, and hopefully will be in for Druid 39.
There was a problem hiding this comment.
Follow-up assessment
Thanks — understood. I rechecked the current Netty 3 source and client lifecycle while reviewing 1 of 1 changed files. I agree this can be an explicit trusted-peer risk decision for Druid-to-Druid traffic. The remaining technical caveat is that Netty 3 is not automatically safe just because the upstream advisory names Netty 4.x: its HttpClientCodec dequeues the request method for a 103 response, while Druid completes every non-chunked response and returns the channel before the final response arrives. A peer that emits 103 can therefore still create the unsafe reuse window.
I’m not reopening the prior finding as a separate merge recommendation given the explicit risk acceptance. Please track it with apache/druid#19567, and consider updating the suppression comment so it describes this as an accepted trusted-peer exposure rather than implying that Netty 3’s different implementation removes the risk.
Reviewed 1 of 1 changed files.
This is an automated review by Codex GPT-5.6-Luna(max)
There was a problem hiding this comment.
@gianm , I have not removed the suppression. Updated the comment instead.
Please let me know if it seems okay.
There was a problem hiding this comment.
Follow-up assessment
I rechecked the current head and the Netty 3 client while reviewing 1 of 1 changed files. The updated comment is not an accurate justification for closing the finding: Druid does not intentionally send HEAD or pipeline requests, but Netty 3's HttpClientCodec still treats a 103 as an empty response and NettyHttpClient returns the channel before the final response, so a peer can create the reuse window. Since @gianm explicitly accepted this as a trusted-peer risk, I am not reopening the prior P1; please update this comment to document that accepted risk and track #19567 rather than claiming that none of the prerequisites apply.
Reviewed 1 of 1 changed files.
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 0 |
| P3 | 0 |
| Total | 1 |
The stale async-http-client suppression was removed and that follow-up is resolved. The Netty 3 response-desynchronization suppression remains unresolved; the inline finding documents why the current Druid integration is still unsafe. No additional issues were found in the current one-file diff.
Reviewed 1 of 1 changed files.
This is an automated review by Codex GPT-5.6-Luna(max)
| <cve>CVE-2026-45674</cve> <!-- Affects netty-resolver-dns which Druid doesn't use; Druid uses JDK DNS resolution --> | ||
| <cve>CVE-2026-48006</cve> <!-- We don't use the Redis codec --> | ||
| <cve>CVE-2026-42585</cve> <!-- Server-side HTTP request smuggling; Druid's HTTP server is Jetty, not Netty --> | ||
| <cve>CVE-2026-42584</cve> <!-- HttpClientCodec response desynchronization in Netty 4.x codec; Druid uses Netty 3.x's HttpClientCodec which has a different implementation --> |
There was a problem hiding this comment.
[P1] Do not suppress the Netty response desynchronization
The “Netty 4.x / different implementation” justification is not safe for Druid's client. In the local Netty 3.10.6 source, HttpClientCodec.Decoder special-cases only status 100 before queue.poll(), while HttpMessageDecoder treats all 1xx statuses (including 103) as empty. NettyHttpClient calls finishRequest() for every non-chunked response and returns the channel to ResourcePool at lines 258-259 and 317-335. Thus a server can send 103, make Druid return and reuse the socket, and deliver the final response after the next request has been written. Remove this CVE from the Netty 3 suppression or explicitly track and mitigate the exposure until the client waits for final responses or is patched/upgraded.
…s, opentelemetry, testcontainers) - CVE-2026-33117 (azure): affects Key Vault Keys local crypto path; Druid uses Azure for blob storage only - CVE-2026-49845, CVE-2026-53561, CVE-2026-55976 (hive-storage-api): Hive server CVEs; Druid only uses Murmur3 hash util from this jar - CVE-2026-54512, CVE-2026-54513, CVE-2026-68497 (jackson-databind): affect jackson shaded inside hadoop-client-runtime and parquet-jackson jars; not Druid's own jackson - CVE-2026-2332, CVE-2026-10050 (jetty): affect Jetty shaded in hadoop-client-runtime; not Druid's own Jetty 12.x - CVE-2026-56741, CVE-2026-56740 (jline): JLine Telnet server DoS; shaded in hadoop-client-runtime, Druid exposes no Telnet endpoint - CVE-2026-24051, CVE-2026-39883, CVE-2026-29181 (opentelemetry): false positive — all affect the Go SDK, not the Java artifact - CVE-2026-40542, CVE-2026-71290, CVE-2026-54399, CVE-2026-54428 (httpclient5/httpcore5): shaded in docker-java-transport-zerodep which is test-scoped only Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 4 |
| P3 | 0 |
| Total | 5 |
This is an automated review by Codex GPT-5.6-Luna(max)
| file name: hadoop-client-runtime-3.5.0.jar (shaded jackson-databind 2.18.6) | ||
| parquet-jackson-1.18.0.jar (shaded jackson-databind 2.22.1) | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/com\.fasterxml\.jackson\.core/jackson-databind@(2\.18\.|2\.22\.).*$</packageUrl> |
There was a problem hiding this comment.
[P1] Scope Jackson suppressions to the shaded jars
packageUrl is the only matching condition here; the file name in <notes> is informational, so this regex also matches Druid's normal com.fasterxml.jackson.core:jackson-databind dependency. The root POM manages that dependency at 2.22.2, which falls in 2.22.*, meaning these CVEs are suppressed for the production Jackson artifact as well as the shaded 2.18.6/2.22.1 copies. Please scope this to the Hadoop/Parquet container (or otherwise distinguish the shaded artifact) instead of suppressing the same GAV globally.
There was a problem hiding this comment.
Agreed that this should be written to only match the shaded jars.
There was a problem hiding this comment.
Thanks — I rechecked the current head, and this remains unresolved. At line 959, packageUrl is still the only matching condition, and the regex matches pkg:maven/com.fasterxml.jackson.core/jackson-databind@2.22.2, which is Druid's ordinary dependency (root pom.xml manages Jackson at 2.22.2) as well as the shaded findings described in the notes. The file name text inside <notes> is documentation only and does not scope a suppression. Please add a file/component matcher (or otherwise distinguish the shaded Hadoop/Parquet jars) before merging.
Reviewed 1 of 1 changed files.
There was a problem hiding this comment.
updated to use filePath regex that matches shaded jars only.
There was a problem hiding this comment.
Follow-up assessment
I rechecked the current head while reviewing 1 of 1 changed files. This filePath pattern still does not match the shaded artifacts: hadoop-client-runtime-3.5.0.jar is a flat fat JAR with relocated classes and no nested .../jackson-databind-*.jar path, and parquet-jackson-1.18.0.jar declares jackson-databind as a separate dependency instead of embedding it. Dependency-Check matches filePath against the dependency's actual file path, so the expression at line 882 (and the analogous expressions at lines 898 and 910) matches neither case; the CVEs remain unsuppressed or the scope is not the intended one. Please scope using the actual paths/component relationships and recheck all three rules.
Reviewed 1 of 1 changed files.
| <notes><![CDATA[ | ||
| file name: azure-core-1.58.1.jar azure-core-http-netty-1.16.5.jar azure-identity-1.18.4.jar azure-json-1.5.1.jar | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/com\.azure/azure-.*@.*$</packageUrl> |
There was a problem hiding this comment.
[P2] Do not match every Azure artifact
The regex is broader than the artifacts listed in the notes: it matches com.azure:azure-security-keyvault-keys, which is the package named by CVE-2026-33117, in addition to azure-core/azure-identity/storage clients. Because notes do not constrain the rule, any future Key Vault Keys dependency would have this critical CVE silently suppressed. Match the specific non-vulnerable GAVs (or the containing jar) instead.
| <notes><![CDATA[ | ||
| file name: hadoop-client-runtime-3.5.0.jar (shaded jetty-http/jetty-io 9.4.58.v20250814) | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/org\.eclipse\.jetty/jetty-(http|io)@9\.4\.58.*$</packageUrl> |
There was a problem hiding this comment.
[P2] Include the Jetty artifact for CVE-2026-10050
This pattern matches only purls for jetty-http and jetty-io. CVE-2026-10050 is associated with Jetty's jetty-security artifact and its DigestAuthentication code in jetty-client; neither identifier matches this alternation. Thus the second CVE remains unsuppressed even if it is reported from the embedded Hadoop runtime. Split the CVEs and match the actual reported embedded artifact(s), or the containing Hadoop jar.
| <notes><![CDATA[ | ||
| file name: hadoop-client-runtime-3.5.0.jar (shaded jline 3.9.0) | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/org\.jline/jline@.*$</packageUrl> |
There was a problem hiding this comment.
[P2] Match JLine's remote Telnet module
The affected Maven package for both CVEs is org.jline:jline-remote-telnet, not org.jline:jline. Hadoop's 3.5.0 runtime embeds the jline-remote-telnet module and its shaded Telnet classes, and Dependency-Check analyzes embedded Maven POMs as separate package identifiers. This exact jline purl therefore misses the reported package, so the suppression will not clear these findings. Match jline-remote-telnet (and any other reported embedded modules) explicitly.
| <notes><![CDATA[ | ||
| file name: docker-java-transport-zerodep-3.7.1.jar (shaded httpclient5 5.5.1 and httpcore5 5.3.6) | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/org\.apache\.httpcomponents\.(client5/httpclient5|core5/httpcore5)@.*$</packageUrl> |
There was a problem hiding this comment.
[P2] Include httpcore5-h2 in the suppression
CVE-2026-54428 is published for org.apache.httpcomponents.core5:httpcore5-h2, but the alternation covers only core5/httpcore5 and omits core5/httpcore5-h2. docker-java-transport-zerodep-3.7.1.jar embeds httpcore5-h2 5.3.6 separately, so the HTTP/2 CVE remains reported. Add core5/httpcore5-h2 to the rule or scope it to the containing test jar.
…p-client-runtime The packageUrl regexes only matched specific submodules (jetty-http, jetty-io, jline), missing others like jetty-xml and jline-style that appear in the same shaded hadoop-client-runtime jar. Broadened to jetty-.* and jline.* to cover all submodules. The suppression rationale is unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… in hadoop-client-runtime The packageUrl regex only matched artifacts under the org.eclipse.jetty group, missing subgroups like org.eclipse.jetty.websocket. Broadened the pattern to cover all org.eclipse.jetty.* Maven groups at version 9.4.58.x shaded inside hadoop-client-runtime. Suppression rationale is unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 2 |
| P3 | 0 |
| Total | 3 |
Reviewed 1 of 1 changed files.
I reassessed prior findings against the current head. The Jetty and JLine package-pattern issues are resolved by the updated regexes. I am not carrying the earlier Netty-3 finding because the upstream advisory's affected range starts at Netty 4.x and the current Druid dependency is Netty 3.10.6. The three inline findings below remain actionable.
This is an automated review by Codex GPT-5.6-Luna(max)
| file name: hadoop-client-runtime-3.5.0.jar (shaded jackson-databind 2.18.6) | ||
| parquet-jackson-1.18.0.jar (shaded jackson-databind 2.22.1) | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/com\.fasterxml\.jackson\.core/jackson-databind@(2\.18\.|2\.22\.).*$</packageUrl> |
There was a problem hiding this comment.
[P1] Scope Jackson suppressions to shaded jars
packageUrl is the only matcher here; the file name in <notes> is informational. This regex therefore also matches Druid's ordinary com.fasterxml.jackson.core:jackson-databind artifact. The root POM manages that artifact at 2.22.2, which matches 2.22.*, so these CVEs are suppressed for the production Jackson GAV as well as the shaded 2.18.6/2.22.1 copies described in the notes. Scope each suppression to the shaded package/container (or use an exact GAV/version) so a real finding in Druid's own Jackson is not hidden.
| <notes><![CDATA[ | ||
| file name: azure-core-1.58.1.jar azure-core-http-netty-1.16.5.jar azure-identity-1.18.4.jar azure-json-1.5.1.jar | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/com\.azure/azure-.*@.*$</packageUrl> |
There was a problem hiding this comment.
[P2] Do not match every Azure artifact
^pkg:maven/com\.azure/azure-.*@.*$ also matches com.azure:azure-security-keyvault-keys, the package named by CVE-2026-33117, not just the azure-core/identity/storage jars listed in the notes. The notes do not constrain the rule. If that critical Key Vault Keys artifact is added or appears transitively, Dependency-Check will suppress the real CVE as if it were a false positive. Match the explicit non-vulnerable GAVs/versions or the containing shaded jar instead.
| <notes><![CDATA[ | ||
| file name: docker-java-transport-zerodep-3.7.1.jar (shaded httpclient5 5.5.1 and httpcore5 5.3.6) | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/org\.apache\.httpcomponents\.(client5/httpclient5|core5/httpcore5)@.*$</packageUrl> |
There was a problem hiding this comment.
[P2] Include httpcore5-h2 in the suppression
CVE-2026-54428 is published for org.apache.httpcomponents.core5:httpcore5-h2, but this alternation only matches core5/httpcore5 and client5/httpclient5. docker-java-transport-zerodep-3.7.1.jar embeds the HTTP/2 module separately at 5.3.6, so the rule misses the component that this CVE is attached to and the finding remains unsuppressed. Add core5/httpcore5-h2 or scope the rule to the containing test jar.
| </suppress> | ||
|
|
||
| <suppress> | ||
| <!-- Netty 4.x CVEs for codecs/features that Druid never activates. |
There was a problem hiding this comment.
We shouldn't need any Netty 4 suppressions in the PR to master, since we should be using an up-to-date Netty 4. Please remove the Netty 4 suppressions and validate that things still pass.
There was a problem hiding this comment.
removed netty4 suppresions.
| get/set/delete commands. Druid does not run or embed a Memcached server process. | ||
|
|
||
| The version string "1.2.4" of the Java client JAR is incorrectly matched by the scanner | ||
| against CPE cpe:2.3:a:memcached:memcached:1.2.4 (the C memcached server), causing false positives. |
There was a problem hiding this comment.
Is there a way we can deal with this by fixing the scanner rather than adding individual CVE suppressions? Like, can we exclude memcached:memcached?
| library; all CVEs below are vulnerabilities in the Memcached server C code. | ||
| Druid acts as a Memcached client and is not affected by server-side CVEs. --> | ||
| <notes><![CDATA[ | ||
| file name: elasticache-java-cluster-client-1.2.4.jar |
There was a problem hiding this comment.
Why do we need to list the elasticache false positives in two separate <suppress> blocks? Can they be put into a single block?
| <notes><![CDATA[ | ||
| file name: package-lock.json (pkg:npm/uuid@7.0.3) | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:npm/uuid@.*$</packageUrl> |
There was a problem hiding this comment.
Rather than suppress in master, can we update uuid?
There was a problem hiding this comment.
Added a note for now to unblock this PR.
Will take up the upgrade in a separate PR and then remove this suppression.
| <suppress> | ||
| <!-- CVE-2026-33117: Vulnerability in Azure SDK for Java's Key Vault Keys local cryptographic verification path. | ||
| Druid's azure-extensions use azure-core/azure-identity for blob storage auth only; Druid does not use | ||
| azure-keyvault-keys or the local cryptography client path that contains the vulnerability. --> |
There was a problem hiding this comment.
If we don't use azure-keyvault-keys at all, it's better to exclude the dependency rather than suppress the CVE.
There was a problem hiding this comment.
We are not pulling in the dependency azure-keyvault-keys anywhere but apparently, the CVE gets flagged for other azure artifacts too (like azure-core, azure-identity, etc).
I have updated the comment to avoid confusion.
| <notes><![CDATA[ | ||
| file name: hadoop-client-runtime-3.5.0.jar (shaded jline 3.9.0) | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/org\.jline/jline.*@.*$</packageUrl> |
There was a problem hiding this comment.
Should be updated to only apply to the shaded jar.
| <notes><![CDATA[ | ||
| file name: hadoop-client-runtime-3.5.0.jar (shaded org.eclipse.jetty* 9.4.58.v20250814) | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/org\.eclipse\.jetty(\.[^/]+)?/.*@9\.4\.58.*$</packageUrl> |
There was a problem hiding this comment.
Should be updated to only apply to the shaded jar.
| These CVEs affect jackson-databind shaded inside hadoop-client-runtime-3.5.0.jar and | ||
| parquet-jackson-1.18.0.jar — not Druid's own jackson-databind (2.22.x). Druid cannot | ||
| upgrade the jackson version inside these third-party shaded jars. Druid's own usage of | ||
| @JsonTypeInfo uses a custom StrictTypeIdResolver that is not affected by these bypass paths. |
There was a problem hiding this comment.
Is this comment entirely true? I don't think we typically use a custom StrictTypeIdResolver. We do typically use Id.NAME for type info though, which isn't vulnerable to this class of problems.
Anyway, I also think this is over-arguing the point. It can't be relevant both that these Jacksons are only used by shaded Hadoop jars, and that Druid uses @JsonTypeInfo in a safe way. If the former is true then the latter is irrelevant.
There was a problem hiding this comment.
updated the comment.
| file name: netty-codec-protobuf-4.2.15.Final.jar netty-transport-4.2.15.Final.jar (and other netty-*.jar at 4.2.x) | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/io\.netty/netty-.*@4\.2\..*$</packageUrl> | ||
| <cve>CVE-2026-56819</cve> <!-- HTTP/2 decompressor ByteBuf leak: Druid does not run a Netty 4.x HTTP/2 server --> |
There was a problem hiding this comment.
We might do so in the gRPC extension. But I think it doesn't matter anyway, because none of these suppressions should need to be here, because we should be on the latest Netty 4.
There was a problem hiding this comment.
removed netty4 suppresions.
| <cve>CVE-2026-45674</cve> <!-- Affects netty-resolver-dns which Druid doesn't use; Druid uses JDK DNS resolution --> | ||
| <cve>CVE-2026-48006</cve> <!-- We don't use the Redis codec --> | ||
| <cve>CVE-2026-42585</cve> <!-- Server-side HTTP request smuggling; Druid's HTTP server is Jetty, not Netty --> | ||
| <cve>CVE-2026-42584</cve> <!-- HttpClientCodec response desynchronization in Netty 4.x codec; Druid uses Netty 3.x's HttpClientCodec which has a different implementation --> |
There was a problem hiding this comment.
I believe this suppression is defensible but for a different reason. We use Netty 3 only for NettyHttpClient, which is only used for Druid-to-Druid server communication, and Druid servers don't pipeline requests (we wait for the response first), don't send HEAD to each other, and shouldn't be replying to each other with 1xx's. The advisory says all three are required.
Anyway, WTB #19567.
FrankChen021
left a comment
There was a problem hiding this comment.
🟡 Changes recommended
I reassessed the current head from the incremental diff first, then the complete merge-base diff and surrounding dependency/client code. The new shaded-JAR path expressions remain actionable because they do not match the actual layouts of the artifacts described by their comments; this affects the Jackson rule and the analogous Jetty and JLine rules. The previously discussed Netty concern is not reopened as a separate merge recommendation because the maintainers explicitly accepted the trusted-peer risk, but the updated comment still needs to document that decision accurately; I handled that in an inline follow-up.
Reviewed 1 of 1 changed files. I also checked the relevant Maven dependency declarations, local artifact layouts, Dependency-Check filePath matching behavior, and the Netty 3 response/channel lifecycle. Narrow validation passed: git diff --check 33f7f778dfc2d8b06ca7c1d7a93f54f162526cee...HEAD and xmllint --noout owasp-dependency-check-suppressions.xml. No builds, tests, dependency installs, fetches, or formatters were run.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 0 |
| P3 | 0 |
| Total | 1 |
This is an automated review by Codex GPT-5.6-Luna(max)
After addressing the findings or replying to the comments, you can request another review from me to trigger a new automated review.
| file name: hadoop-client-runtime-3.5.0.jar (shaded jackson-databind 2.18.6) | ||
| parquet-jackson-1.18.0.jar (shaded jackson-databind 2.22.1) | ||
| ]]></notes> | ||
| <filePath regex="true">.*/(hadoop-client-runtime|parquet-jackson)-[0-9][^/]*\.jar[!/]+jackson-databind-[0-9].*\.jar$</filePath> |
There was a problem hiding this comment.
[P1] Scope the shaded-JAR suppressions to actual paths
This filePath pattern requires a dependency path shaped like hadoop-client-runtime-3.5.0.jar/.../jackson-databind-.jar. The artifacts named in the comment do not have that layout: hadoop-client-runtime-3.5.0.jar is a flat shaded JAR with relocated Jackson/Jetty/JLine classes and no nested JAR entries, while parquet-jackson-1.18.0.jar declares jackson-databind as a separate dependency rather than embedding it. Dependency-Check matches filePath against the dependency's actual file path, so the Hadoop finding has the outer hadoop-client-runtime path and the Parquet finding has a standalone Jackson path; neither matches this expression. The analogous Jetty and JLine expressions at lines 898 and 910 fail for the same reason. As a result, these CVEs remain unsuppressed and the release-profile dependency-check can still fail, or the rule is not scoped to the intended shaded code. Please scope each suppression using the actual scanner paths/component relationships and verify the result against a generated report.
|
There are some failures in the check, possibly due to #20236 (comment). |
|
@gianm , thanks for the thorough review! |
FrankChen021
left a comment
There was a problem hiding this comment.
🟡 Changes recommended
I reassessed the current head from the supplied incremental diff first, then reviewed the authoritative complete diff and surrounding dependency, artifact, and Dependency-Check code. The incremental packet contains unrelated changes from the branch's merge of master; the current merge-base diff is one changed file. The rewritten shaded-JAR filePath rules now match the embedded META-INF/maven POM paths present in the Hadoop and Parquet artifacts, so the prior shaded-path finding is resolved. The earlier Netty and stale async-http-client findings are not carried forward.
Reviewed 1 of 1 changed files. Two actionable suppression-scope issues remain in the inline findings above.
Validation: xmllint --noout owasp-dependency-check-suppressions.xml passed, and git diff --check bba74f6cb28eb091c41835b6e26d270b535a0c73...fc0fb6e6787531d113f83d810dddb9e490d64a39 -- owasp-dependency-check-suppressions.xml passed. I also read the current PR checks; the security-vulnerabilities job fails earlier in druid-google-extensions on unchanged dev.cel:protobuf@0.13.0 / CVE-2022-3171, before the testcontainers module is analyzed. No builds, tests, dependency installs, fetches, or formatters were run.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 2 |
| P3 | 0 |
| Total | 2 |
This is an automated review by Codex GPT-5.6-Luna(max)
After addressing the findings or replying to the comments, you can request another review from me to trigger a new automated review.
| <notes><![CDATA[ | ||
| file name: azure-core-1.58.1.jar azure-core-http-netty-1.16.5.jar azure-identity-1.18.4.jar azure-json-1.5.1.jar | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/com\.azure/azure-.*@.*$</packageUrl> |
There was a problem hiding this comment.
[P2] Narrow the Azure suppression to non-vulnerable artifacts
This packageUrl regex matches every com.azure:azure-* artifact, including com.azure:azure-security-keyvault-keys, which is the artifact named by CVE-2026-33117. Because packageUrl is an active suppression condition and the notes do not constrain the match, adding that real Key Vault dependency would silently suppress the vulnerability as a false positive. Match the four listed non-Key-Vault GAVs (or their containing files) explicitly.
| <notes><![CDATA[ | ||
| file name: docker-java-transport-zerodep-3.7.1.jar (shaded httpclient5 5.5.1 and httpcore5 5.3.6) | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/org\.apache\.httpcomponents\.(client5/httpclient5|core5/httpcore5)@.*$</packageUrl> |
There was a problem hiding this comment.
[P2] Include the embedded httpcore5-h2 module
docker-java-transport-zerodep-3.7.1.jar embeds separate POMs for org.apache.httpcomponents.core5:httpcore5 and org.apache.httpcomponents.core5:httpcore5-h2. This alternation matches only core5/httpcore5, so a CVE-2026-54428 finding attached to httpcore5-h2 cannot be suppressed and will remain in the security scan once druid-testcontainers is analyzed. Add core5/httpcore5-h2 or scope the rule to the containing test jar.
|
@FrankChen021 , I think we might be hard pressed to upgrade zstd-jni to 1.5.7-14 or higher, since it has been reported to have some vulnerabilities which seem applicable to Druid as well. https://github.com/apache/druid/actions/runs/34683579050/job/103526503953?pr=20236 An upgrade of this library was previously considered in the below PRs but postponed since zstd-jni versions post 1.5.7-12 do not support legacy I think our option at this point is to upgrade the library and document this as a breaking change. |
|
@kfaraz I investigated both CVEs against Druid and every zstd-jni consumer I found in the packaged distribution. Although
The identified consumers and their actual call paths are:
I also scanned the Parquet 1.18.0 and 1.18.1 artifacts, and the other dependency jars above, for references to the CVE-specific symbols. None calls the unsafe raw methods or constructs Therefore, I found no reachable path for either CVE through Druid or its bundled dependency callers. The vulnerable methods remain present in the jar, so a version-based scanner correctly reports the component, but that is different from application-level exploitability. Based on this analysis, my final suggestion is that suppressing these two findings with the API-reachability rationale appears reasonable in one more release druid 38 and 39, and call out deprecation of old zstd frames. And we upgrade this dependency in Druid 40. |
FrankChen021
left a comment
There was a problem hiding this comment.
🟡 Changes recommended
I started with the incremental diff, then reviewed the complete merge-base diff and the surrounding dependency, artifact, and Dependency-Check configuration for the current head. The new dev.cel:protobuf rule is scoped to a distinct CEL artifact, and the zstd call-path rationale is consistent with the inspected Druid and dependency consumers; the zstd selector itself should still be narrowed to the audited version. The prior shaded-JAR path concern is resolved by the current embedded-POM layouts, and the current checks no longer report those Jackson, Jetty, or JLine rules. The Azure matcher remains broader than the artifacts documented in its notes. The HttpComponents matcher still omits the separately shaded httpcore5-h2 package, which is the unresolved finding currently keeping the security-vulnerabilities job failing.
Reviewed 1 of 1 current changed files: /Users/frankchen/.codex/mergelens/worktrees/apache__druid__pr-20236/owasp-dependency-check-suppressions.xml.
Validation: xmllint --noout passed; git diff --check passed; inspected the current PR checks and security job log, which reports CVE-2026-54399 and CVE-2026-54428 for docker-java-transport-zerodep-3.7.1.jar/META-INF/maven/org.apache.httpcomponents.core5/httpcore5-h2/pom.xml. No local build, test, dependency install, fetch, or source edit was performed.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 3 |
| P3 | 0 |
| Total | 3 |
This is an automated review by Codex GPT-5.6-Luna(max)
After addressing the findings or replying to the comments, you can request another review from me to trigger a new automated review.
| <notes><![CDATA[ | ||
| file name: azure-core-1.58.1.jar azure-core-http-netty-1.16.5.jar azure-identity-1.18.4.jar azure-json-1.5.1.jar | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/com\.azure/azure-.*@.*$</packageUrl> |
There was a problem hiding this comment.
[P2] Narrow the Azure suppression to the reviewed artifacts
Finding: This packageUrl matcher suppresses CVE-2026-33117 for every com.azure artifact, not just the four jars listed in the notes. It therefore also matches com.azure:azure-security-keyvault-keys, the artifact associated with the Key Vault Keys verification vulnerability, so a future dependency on that client would have its real finding silently suppressed.
Suggestion: Replace the wildcard with exact packageUrl rules for the reviewed Azure artifacts (or a containing-jar/filePath matcher), and keep the vulnerable Key Vault artifacts outside the suppression scope.
| <notes><![CDATA[ | ||
| file name: docker-java-transport-zerodep-3.7.1.jar (shaded httpclient5 5.5.1 and httpcore5 5.3.6) | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/org\.apache\.httpcomponents\.(client5/httpclient5|core5/httpcore5)@.*$</packageUrl> |
There was a problem hiding this comment.
[P2] Include the separately shaded httpcore5-h2 artifact
Finding: The alternation matches org.apache.httpcomponents.core5:httpcore5 but not org.apache.httpcomponents.core5:httpcore5-h2. docker-java-transport-zerodep-3.7.1.jar embeds httpcore5-h2 separately, and the current security-vulnerabilities job still reports CVE-2026-54399 and CVE-2026-54428 for that path, so this suppression does not make the documented test-only dependency clean.
Suggestion: Add core5/httpcore5-h2 to the packageUrl alternation, or scope the CVEs to the verified docker-java shaded POM paths, then rerun the dependency-check job.
| <notes><![CDATA[ | ||
| file name: zstd-jni-1.5.7-11.jar | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/com\.github\.luben/zstd-jni@.*$</packageUrl> |
There was a problem hiding this comment.
[P2] Scope the zstd suppressions to the audited version
Finding: The rule suppresses both zstd CVEs for every com.github.luben:zstd-jni version, while its notes identify only zstd-jni-1.5.7-11 and state that the fixes arrive in 1.5.7-14. A future downgrade or dependency-resolution change to another affected zstd-jni version would therefore be hidden without the call-path review documented here.
Suggestion: Match the documented zstd-jni version explicitly (for example, 1.5.7-11), or use a verified containing-file/path matcher, and remove or revise the rule when upgrading to the fixed version.
Thanks for the response, @FrankChen021 ! |
FrankChen021
left a comment
There was a problem hiding this comment.
🟡 Changes recommended
I started with the supplied incremental diff, then reconciled it against the complete merge-base diff and the current PR-head worktree. The incremental delta also contains the two-file FixedIndexed change from the master commit d480d66e; those files are not in the current merge-base changed-file set and were not treated as PR findings. The current httpcore5-h2 suppression now covers the artifact reported by the prior check, and the earlier shaded-JAR path findings remain resolved. Two suppression-scope issues remain: the Azure rule is group-wide although its rationale covers four artifacts, and the zstd rule is version-unbounded although its reachability analysis covers 1.5.7-11.
Reviewed 1 of 1 current changed files: /Users/frankchen/.codex/mergelens/worktrees/apache__druid__pr-20236/owasp-dependency-check-suppressions.xml.
Validation: xmllint --noout /Users/frankchen/.codex/mergelens/worktrees/apache__druid__pr-20236/owasp-dependency-check-suppressions.xml passed; git diff --check d480d66e7fe52ef5daa46eb1af9f33f954be9b7c HEAD passed. No builds, tests, dependency installs, fetches, formatters, or source edits were run.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 2 |
| P3 | 0 |
| Total | 2 |
This is an automated review by Codex GPT-5.6-Luna(max)
After addressing the findings or replying to the comments, you can request another review from me to trigger a new automated review.
| <notes><![CDATA[ | ||
| file name: azure-core-1.58.1.jar azure-core-http-netty-1.16.5.jar azure-identity-1.18.4.jar azure-json-1.5.1.jar | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/com\.azure/azure-.*@.*$</packageUrl> |
There was a problem hiding this comment.
[P2] Azure suppression matches every artifact
Finding: This packageUrl regex suppresses CVE-2026-33117 for every Maven artifact whose ID starts with azure- under com.azure, while the rationale names only azure-core, azure-core-http-netty, azure-identity, and azure-json. The current Azure extension also declares azure-storage-blob, azure-storage-blob-batch, and azure-storage-common, and a future Azure dependency with the affected Key Vault implementation would have its finding silently discarded.
Suggestion: Constrain the matcher to the four audited artifact IDs (or add an equivalent artifact/path scope) and keep the exception version- or component-specific.
| <notes><![CDATA[ | ||
| file name: zstd-jni-1.5.7-11.jar | ||
| ]]></notes> | ||
| <packageUrl regex="true">^pkg:maven/com\.github\.luben/zstd-jni@.*$</packageUrl> |
There was a problem hiding this comment.
[P2] Zstd suppression is version-unbounded
Finding: The reachability analysis and notes are for zstd-jni-1.5.7-11, but this regex suppresses both CVEs for every zstd-jni version. Changing the pinned dependency or adding another resolved version would therefore bypass the required re-audit even when the affected raw APIs or a different implementation become reachable.
Suggestion: Match the audited zstd-jni version explicitly (or otherwise scope the exception to the reviewed call paths) and revisit it when the dependency is upgraded.
Created with the same changes as in #20235 since that PR was from a fork which couldn't use secrets (NVD_API_KEY) to run security vulnerabilities check.
This patch suppresses some CVEs encountered while preparing the release of Apache Druid 38.0.0
https://github.com/apache/druid/actions/runs/31012362900/job/92327549691
The patch is mostly Claude-generated.
This PR has: