CAMEL-24423/24442/24449/24453/24455/24475: untrusted input handling fixes, backport to camel-4.22.x - #25940
Conversation
…h, not by prefix (apache#25833) isHttpProxy() tested path.startsWith(PROXY_PATH), so any endpoint whose path merely began with "proxy" - proxyStats, proxy-health, proxying - was treated as the documented platform-http:proxy endpoint. That is not only a naming curiosity: getPath() returns "/" for such an endpoint, making it a catch-all, and VertxPlatformHttpConsumer.handleProxy() sets Exchange.HTTP_HOST from the request's own Host header so a bridging producer forwards there. A route author naming an endpoint proxyStats got a catch-all whose forward target came from the caller. Compare for equality. The check is deliberately strict rather than tolerating a leading slash: platform-http:/proxy did not select proxy mode before and still does not, so tightening this can never turn an endpoint into a proxy that was not already one. The test asserts that, so the check is not loosened later by mistake. Every platform-http:proxy usage in the tree - the component docs, PlatformHttpProxyTest, VertxPlatformHttpProxyTest, VertxPlatformHttpsProxyTest - already uses the exact path. Signed-off-by: Andrea Cosentino <ancosen@gmail.com> (cherry picked from commit 7abf13b) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…insensitively when suppressing the echo (apache#25831) enhanceHeaderFilterStrategyToSkipHttpRequestHeaders() keeps common request headers - Authorization, Cookie, Proxy-Authorization and the rest of COMMON_HTTP_REQUEST_HEADERS - from being echoed back on the response. The lookup was Set.contains(headerName) against a canonically capitalised Set.of(...), while exchange headers keep the casing of the inbound request: VertxPlatformHttpConsumer populates them from the Vert.x MultiMap as received. HTTP/2 requires field names to be lower case, so on an HTTP/2 request the names are authorization, cookie and so on, none of which matched. The suppression therefore never fired for HTTP/2 traffic, nor for any client that varied the casing, and VertxPlatformHttpSupport.copyMessageHeadersToResponse wrote the headers to the response. Hold the set in a TreeSet ordered by String.CASE_INSENSITIVE_ORDER so the comparison no longer depends on how the client spelled the name. Signed-off-by: Andrea Cosentino <ancosen@gmail.com> (cherry picked from commit 126c79b) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ed defaultInstance (apache#25823) * CAMEL-24442: camel-thrift - unmarshal into a copy instead of the shared defaultInstance ThriftDataFormat.unmarshal() deserialized into the defaultInstance field and returned that same object. The data format is shared by every exchange on the route, and Thrift's TBase.read() assigns only the fields present in the incoming bytes without clearing the object first, so: - a message that omitted an optional field kept the value left there by the previous message - deterministic, no concurrency needed; - concurrent unmarshals interleaved field writes into the one object; - every in-flight body was literally the same reference. Deserialize into defaultInstance.deepCopy() and return that. ProtobufDataFormat already builds a new instance per unmarshal. As a side effect defaultInstance is left untouched and now works as the template its name promises: values preset on it are visible on every message, where before the first message overwrote them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com> * CAMEL-24442: Clear copied Thrift instance before unmarshal --------- Signed-off-by: Andrea Cosentino <ancosen@gmail.com> (cherry picked from commit c52d3cf) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ing it to headers (apache#25819) TikaProducer.convertMetadataToHeaders() copied every metadata name produced by the parse straight onto the Camel message. Those names come out of the document itself, so a document could ask for any header name at all, including names in the Camel-internal namespace - an HTML <meta name="CamelFileName" content="../../x"/> reached the message as CamelFileName and would then be picked up by a later file: producer. Filter the names the same way a consumer filters names supplied by an external sender: a DefaultHeaderFilterStrategy with lowerCase matching and inFilterStartsWith of Camel, camel and org.apache.camel. A filtered name is skipped and logged at DEBUG. Metadata outside that namespace is mapped exactly as before. Filtering rather than prefixing all parsed metadata keeps the change small enough to backport; prefixing would rename every header the component produces today. Signed-off-by: Andrea Cosentino <ancosen@gmail.com> (cherry picked from commit b6f6b47) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e hardened XML parser (apache#25683) XPathBuilder handed an InputSource straight to XPathExpression, which builds a DocumentBuilder of its own with the JDK defaults - so documentType=InputSource (and SAXSource) accepted a DOCTYPE declaration and resolved external entities, while the default documentType of Document did not. All four evaluation sites now convert through the type converter, reusing the same hardened DocumentBuilderFactory the default document type already goes through. This adds no document parse: evaluate(InputSource) already built a full DOM internally. (cherry picked from commit 1ead256) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
… service that uses it SagaProcessor.getCurrentSagaCoordinator() fell back to the unprefixed Long-Running-Action message header whenever the exchange's internal saga state was missing, so it could pick up a coordinator id from an unrelated caller and join that exchange to the wrong saga under AUTO completion. That fallback was added for LRA protocol interoperability (CAMEL-23469), but applied unconditionally to every saga service, including the default InMemorySagaService, which has no external coordinator to interoperate with. Adds CamelSagaService.isLongRunningActionHeaderSupported(), defaulting to false, and only consults the header when the configured service opts in. LRASagaService overrides it to true, preserving the existing interoperability. A custom CamelSagaService joining sagas via the header must now override this method. KafkaSagaIT is updated to advertise support since its saga id only survives a Kafka round-trip through the header. Includes an upgrade-guide entry for 4.23. Closes apache#25828 (cherry picked from commit dfde003) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
There was a problem hiding this comment.
Clean cherry-pick backport of six already-reviewed untrusted-input-handling fixes to camel-4.22.x.
All six original PRs were previously reviewed and approved:
- #25833 (CAMEL-24455, platform-http proxy path matching) — APPROVE
- #25831 (CAMEL-24453, platform-http header echo case sensitivity) — APPROVE
- #25823 (CAMEL-24442, thrift unmarshal isolation) — APPROVE
- #25819 (CAMEL-24423, tika metadata header filtering) — APPROVE
- #25683 (CAMEL-24475, xpath InputSource hardened parser) — APPROVE
- #25828 (CAMEL-24449, LRA saga header restriction) — APPROVE
Straight cherry-picks with no manual conflict resolution. 16 files across 7 modules.
📋 PR Metadata
| Aspect | Current | Suggested |
|---|---|---|
| Labels | components, components-kafka, core |
+ bug |
| Milestone | (none) | 4.22.0 |
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Guillaume Nodet
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 555 tested, 29 compile-only — current: 555 all testedMaveniverse Scalpel detected 584 affected modules (current approach: 555).
|
Backport to
camel-4.22.xof six fixes already reviewed and merged onmain. Each is a case where data arriving from outside the route reached somewhere it should not have.7abf13b2)126c79bd)c52d3cfa)b6f6b470)1ead256f)dfde0031)Straight cherry-picks, applied in the order they merged on
main, with no manual conflict resolution. The upgrade-guide entries are not included: the guides for every line live onmain.Worth calling out for a patch release:
documentType=InputSourcein camel-xpath now goes through the same hardened parser as the default document type, so a DOCTYPE in the payload is refused rather than resolved. The sameDocumentBuilderFactorysystem properties that relaxed the default type still relax this one.Built and tested per module on this branch (
core,camel-lra,camel-kafka,camel-platform-http,camel-thrift,camel-tika,camel-xpath), including the new and touched tests:PlatformHttpEndpointProxyPathTest,PlatformHttpEndpointHeaderEchoTest,ThriftUnmarshalIsolationTest,TikaMetadataHeaderFilterTest,XPathFeatureTest,SagaHeaderCannotSelectCoordinatorTest.Claude Code on behalf of oscerd