[
https://issues.apache.org/jira/browse/CAMEL-24509?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrea Cosentino updated CAMEL-24509:
-------------------------------------
Description:
Two related hardening items in camel-mina's object-codec path.
*1. The exchange-holder unmarshal is not gated on {{transferExchange}}*
{{MinaPayloadHelper.setIn()}} / {{setOut()}} unmarshal an inbound
{{DefaultExchangeHolder}} whenever the decoded object happens to be one, with
no check that the endpoint enabled {{transferExchange}}:
{code:java}
public static void setIn(Exchange exchange, Object payload) {
if (payload instanceof DefaultExchangeHolder) {
DefaultExchangeHolder.unmarshal(exchange, (DefaultExchangeHolder)
payload);
{code}
{{DefaultExchangeHolder.unmarshal()}} rebuilds the whole Exchange from the
payload - exchange id, in body, in headers (via {{setHeaders}}, which replaces
the map wholesale, including the {{MINA_*}} headers the consumer set moments
earlier), the out message, and every exchange property.
The equivalent path in camel-jms is gated: {{JmsBinding}} checks
{{isObjectMessageEnabled()}} and calls {{checkDeserializedClass(payload)}}
before it will unmarshal a holder. camel-mina has neither check.
*Reachability is limited, and this is defence in depth rather than a reachable
hole.* The object codec's accept-list refuses an arbitrary class when
{{objectCodecPattern}} is unset, which is the default - verified against
mina-core 2.2.4, where a {{String}} decodes but a {{HashMap}} or a
{{DefaultExchangeHolder}} raises {{ClassNotFoundException}}. Reaching the
helper with a holder therefore requires the operator to have widened
{{objectCodecPattern}} first.
Proposal: gate the unmarshal on the endpoint's {{transferExchange}} setting, so
a decoded holder is treated as an ordinary body unless the endpoint asked for
exchange transfer. This matches camel-jms and leaves documented
{{transferExchange=true}} deployments working.
*2. {{objectCodecPattern}} is not marked as a security-relevant option*
The option widens the deserialization allow-list but carries no {{security}}
marker, unlike {{transferExchange}} on the same configuration class, which is
annotated {{security = "insecure:serialization"}}. Its description also does
not warn about {{*}}, while three historical upgrade guides (4.4, 4.8, 4.10)
tell readers "You can use {{*}} to accept all patterns".
Proposal: mark the option {{security = "insecure:serialization"}} and extend
its description to say that the pattern should be as narrow as the route needs,
that {{*}} accepts every loadable class from an untrusted peer, and that
leaving it unset is the safest choice. The historical upgrade guides are
release history and are left as-is.
Present on {{main}}, {{camel-4.22.x}} and {{camel-4.18.x}}.
was:
{{MinaPayloadHelper.setIn()}} and {{setOut()}} unmarshal an inbound
{{DefaultExchangeHolder}} unconditionally:
{code:java}
public static void setIn(Exchange exchange, Object payload) {
if (payload instanceof DefaultExchangeHolder) {
DefaultExchangeHolder.unmarshal(exchange, (DefaultExchangeHolder)
payload);
} else {
// normal transfer using the body only
exchange.getIn().setBody(payload);
}
}
{code}
There is no check that the endpoint actually enabled {{transferExchange}}.
Whenever the decoded object happens to be a {{DefaultExchangeHolder}}, the
whole Exchange is rebuilt from it: {{DefaultExchangeHolder.unmarshal()}} sets
the exchange id, the in body, the in headers (via {{setHeaders}}, which
replaces the map wholesale - including the {{MINA_*}} headers the consumer set
moments earlier), the out body and headers, and every exchange property.
The equivalent path in camel-jms is gated. {{JmsBinding}} checks
{{isObjectMessageEnabled()}} and calls {{checkDeserializedClass(payload)}}
before it will unmarshal a holder:
{code:java}
if (message instanceof ObjectMessage objectMessage) {
if (!isObjectMessageEnabled()) {
throw objectMessageDisabled("receiving ObjectMessage");
}
Object payload = objectMessage.getObject();
checkDeserializedClass(payload);
if (payload instanceof DefaultExchangeHolder holder) {
DefaultExchangeHolder.unmarshal(exchange, holder);
{code}
camel-mina has neither gate, and the exposure is wider than in camel-jms
because object decoding is the default rather than an opt-in:
{{MinaConfiguration.textline}} defaults to {{false}}, so {{MinaConsumer}}
installs {{ObjectSerializationCodecFactory}}. The {{objectCodecPattern}}
allow-list does not help here, since {{DefaultExchangeHolder}} lives in the
{{org.apache.camel}} namespace that any reasonable allow-list permits.
Proposal: gate the holder unmarshal on the endpoint's {{transferExchange}}
setting, so a decoded holder is treated as an ordinary body unless the endpoint
asked for exchange transfer. That matches the camel-jms behaviour and leaves
the documented {{transferExchange=true}} deployments working.
Present on {{main}}, {{camel-4.22.x}} and {{camel-4.18.x}} - identical code on
all three ({{MinaPayloadHelper}} lines 56 and 65, {{textline}} default at
{{MinaConfiguration}} line 50).
Summary: camel-mina - gate the exchange-holder unmarshal on
transferExchange and mark objectCodecPattern as security-relevant (was:
camel-mina - an inbound DefaultExchangeHolder is unmarshalled without checking
transferExchange)
> camel-mina - gate the exchange-holder unmarshal on transferExchange and mark
> objectCodecPattern as security-relevant
> --------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-24509
> URL: https://issues.apache.org/jira/browse/CAMEL-24509
> Project: Camel
> Issue Type: Bug
> Components: camel-mina
> Reporter: Andrea Cosentino
> Assignee: Andrea Cosentino
> Priority: Major
> Fix For: 4.23.0
>
>
> Two related hardening items in camel-mina's object-codec path.
> *1. The exchange-holder unmarshal is not gated on {{transferExchange}}*
> {{MinaPayloadHelper.setIn()}} / {{setOut()}} unmarshal an inbound
> {{DefaultExchangeHolder}} whenever the decoded object happens to be one, with
> no check that the endpoint enabled {{transferExchange}}:
> {code:java}
> public static void setIn(Exchange exchange, Object payload) {
> if (payload instanceof DefaultExchangeHolder) {
> DefaultExchangeHolder.unmarshal(exchange, (DefaultExchangeHolder)
> payload);
> {code}
> {{DefaultExchangeHolder.unmarshal()}} rebuilds the whole Exchange from the
> payload - exchange id, in body, in headers (via {{setHeaders}}, which
> replaces the map wholesale, including the {{MINA_*}} headers the consumer set
> moments earlier), the out message, and every exchange property.
> The equivalent path in camel-jms is gated: {{JmsBinding}} checks
> {{isObjectMessageEnabled()}} and calls {{checkDeserializedClass(payload)}}
> before it will unmarshal a holder. camel-mina has neither check.
> *Reachability is limited, and this is defence in depth rather than a
> reachable hole.* The object codec's accept-list refuses an arbitrary class
> when {{objectCodecPattern}} is unset, which is the default - verified against
> mina-core 2.2.4, where a {{String}} decodes but a {{HashMap}} or a
> {{DefaultExchangeHolder}} raises {{ClassNotFoundException}}. Reaching the
> helper with a holder therefore requires the operator to have widened
> {{objectCodecPattern}} first.
> Proposal: gate the unmarshal on the endpoint's {{transferExchange}} setting,
> so a decoded holder is treated as an ordinary body unless the endpoint asked
> for exchange transfer. This matches camel-jms and leaves documented
> {{transferExchange=true}} deployments working.
> *2. {{objectCodecPattern}} is not marked as a security-relevant option*
> The option widens the deserialization allow-list but carries no {{security}}
> marker, unlike {{transferExchange}} on the same configuration class, which is
> annotated {{security = "insecure:serialization"}}. Its description also does
> not warn about {{*}}, while three historical upgrade guides (4.4, 4.8, 4.10)
> tell readers "You can use {{*}} to accept all patterns".
> Proposal: mark the option {{security = "insecure:serialization"}} and extend
> its description to say that the pattern should be as narrow as the route
> needs, that {{*}} accepts every loadable class from an untrusted peer, and
> that leaving it unset is the safest choice. The historical upgrade guides are
> release history and are left as-is.
> Present on {{main}}, {{camel-4.22.x}} and {{camel-4.18.x}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
