This is an automated email from the ASF dual-hosted git repository.
oscerd pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/main by this push:
new 4848699ace04 CAMEL-24433: camel-mail - sanitise the attachment name on
unmarshal and quote it on marshal (#26661)
4848699ace04 is described below
commit 4848699ace04af38b6568d2fda3a766c9f2b916f
Author: Andrea Cosentino <[email protected]>
AuthorDate: Thu Sep 24 09:39:32 2026 +0200
CAMEL-24433: camel-mail - sanitise the attachment name on unmarshal and
quote it on marshal (#26661)
* CAMEL-24433: camel-mail - sanitise the attachment name on unmarshal and
quote it on marshal
Both ends of the same sender-chosen value.
On unmarshal, MimeMultipartDataFormat.getAttachmentKey() took the file name
from
the part, decoded it and used it as-is to identify the attachment.
MailBinding.extractAndNormalizeFileName() already strips control characters
and
reduces the name to a leaf with FileUtil.stripPath; the data format did
neither,
so a name carrying path components survived intact. It now applies the same
normalisation.
On marshal, the outgoing Content-Type header was built by concatenation:
String value = contentType + "; name=" + attachmentFilename;
The extraction side is well sanitised, but a legal file name may still
contain a
semicolon or a double quote, and those are exactly the characters that
matter in
a MIME parameter. The header is now built with ContentType.setParameter, so
ParameterList quotes and escapes the value when it needs to.
Both are covered against the previous code:
* the unmarshalled attachment was named "../../evil.sh" and is now "evil.sh"
* the outgoing header was "image/jpeg; name=report.jpeg;
boundary=--injected",
where getParameter("boundary") returned "--injected" - the file name had
introduced a parameter of its own. It is now
"image/jpeg; name=\"report.jpeg; boundary=--injected\"" with no extra
parameter.
The existing MailContentTypeResolverTest asserts the exact header string
"image/jpeg; name=logo.jpeg" and still passes: ParameterList only quotes
values
containing tspecials, so ordinary names are formatted as before.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
* CAMEL-24433: camel-mail - scope attachment-name stripPath to the file
name and document the ContentType parse change
Addresses review feedback on #26661:
- getAttachmentKey applied FileUtil.stripPath to every key, including one
taken
from Content-ID. A Content-ID local part may legally contain '/', so the
key is
now reduced to a leaf name only when it comes from the sender-chosen file
name;
Content-ID and generated ids are left as-is. Added a test.
- documented in the 4.23 upgrade guide that a custom ContentTypeResolver
returning an unparsable content type now fails fast with a ParseException,
where before the raw string was written into the header (the parse also
closes
a header-injection vector via the attachment file name).
Co-authored-by: Claude Opus 4.8 <[email protected]>
Signed-off-by: Andrea Cosentino <[email protected]>
---------
Signed-off-by: Andrea Cosentino <[email protected]>
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../apache/camel/component/mail/MailBinding.java | 9 +-
.../mime/multipart/MimeMultipartDataFormat.java | 10 +-
.../mail/MailAttachmentFileNameQuotingTest.java | 116 +++++++++++++++++++++
.../multipart/MimeMultipartDataFormatTest.java | 25 +++++
.../resources/multipart-contentid-with-slash.txt | 13 +++
.../test/resources/multipart-traversal-name.txt | 21 ++++
.../ROOT/pages/camel-4x-upgrade-guide-4_23.adoc | 7 ++
7 files changed, 198 insertions(+), 3 deletions(-)
diff --git
a/components/camel-mail/src/main/java/org/apache/camel/component/mail/MailBinding.java
b/components/camel-mail/src/main/java/org/apache/camel/component/mail/MailBinding.java
index 708f6427a494..ddc61548b6a1 100644
---
a/components/camel-mail/src/main/java/org/apache/camel/component/mail/MailBinding.java
+++
b/components/camel-mail/src/main/java/org/apache/camel/component/mail/MailBinding.java
@@ -39,6 +39,7 @@ import jakarta.mail.MessagingException;
import jakarta.mail.Multipart;
import jakarta.mail.Part;
import jakarta.mail.internet.AddressException;
+import jakarta.mail.internet.ContentType;
import jakarta.mail.internet.InternetAddress;
import jakarta.mail.internet.MimeBodyPart;
import jakarta.mail.internet.MimeMessage;
@@ -748,8 +749,12 @@ public class MailBinding {
LOG.trace("Attachment #{}: Using content type
resolver: {} resolved content type as: {}", i,
contentTypeResolver, contentType);
if (contentType != null) {
- String value = contentType + "; name=" +
attachmentFilename;
- messageBodyPart.setHeader("Content-Type", value);
+ // The file name comes from the message being
relayed, so it must go out as a
+ // parameter value rather than be concatenated
into the header. ParameterList
+ // quotes and escapes anything that would
otherwise change the header's structure.
+ ContentType parsed = new ContentType(contentType);
+ parsed.setParameter("name", attachmentFilename);
+ messageBodyPart.setHeader("Content-Type",
parsed.toString());
LOG.trace("Attachment #{}: ContentType: {}", i,
messageBodyPart.getContentType());
}
}
diff --git
a/components/camel-mail/src/main/java/org/apache/camel/dataformat/mime/multipart/MimeMultipartDataFormat.java
b/components/camel-mail/src/main/java/org/apache/camel/dataformat/mime/multipart/MimeMultipartDataFormat.java
index edd685a537b1..f64bdfa545bb 100644
---
a/components/camel-mail/src/main/java/org/apache/camel/dataformat/mime/multipart/MimeMultipartDataFormat.java
+++
b/components/camel-mail/src/main/java/org/apache/camel/dataformat/mime/multipart/MimeMultipartDataFormat.java
@@ -56,6 +56,7 @@ import org.apache.camel.spi.annotations.Dataformat;
import org.apache.camel.support.DefaultDataFormat;
import org.apache.camel.support.ExchangeHelper;
import org.apache.camel.support.MessageHelper;
+import org.apache.camel.util.FileUtil;
import org.apache.camel.util.IOHelper;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -405,6 +406,7 @@ public class MimeMultipartDataFormat extends
DefaultDataFormat {
private String getAttachmentKey(BodyPart bp) throws MessagingException,
UnsupportedEncodingException {
// use the filename as key for the map
String key = bp.getFileName();
+ boolean fromFileName = key != null;
// if there is no file name we use the Content-ID header
if (key == null && bp instanceof MimeBodyPart mimeBodyPart) {
key = mimeBodyPart.getContentID();
@@ -417,7 +419,13 @@ public class MimeMultipartDataFormat extends
DefaultDataFormat {
if (key == null) {
key = UUID.randomUUID() + "@camel.apache.org";
}
- return MimeUtility.decodeText(key);
+ key = MimeUtility.decodeText(key);
+ key = key.replaceAll("[\n\r\t]", "_");
+ // Only a file name is sender-chosen path data. Reduce it to a leaf
name the same way
+ // MailBinding.extractAndNormalizeFileName does, so it cannot carry
path components before it identifies the
+ // attachment. A Content-ID local part may legally contain '/' (RFC
5322 atext) and a generated id never
+ // carries a path, so those are left as-is.
+ return fromFileName ? FileUtil.stripPath(key) : key;
}
@Override
diff --git
a/components/camel-mail/src/test/java/org/apache/camel/component/mail/MailAttachmentFileNameQuotingTest.java
b/components/camel-mail/src/test/java/org/apache/camel/component/mail/MailAttachmentFileNameQuotingTest.java
new file mode 100644
index 000000000000..e336315d8a59
--- /dev/null
+++
b/components/camel-mail/src/test/java/org/apache/camel/component/mail/MailAttachmentFileNameQuotingTest.java
@@ -0,0 +1,116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.component.mail;
+
+import java.util.Map;
+
+import jakarta.activation.DataHandler;
+import jakarta.activation.FileDataSource;
+import jakarta.mail.internet.ContentType;
+
+import org.apache.camel.Endpoint;
+import org.apache.camel.Exchange;
+import org.apache.camel.Producer;
+import org.apache.camel.attachment.AttachmentMessage;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.mail.Mailbox.MailboxUser;
+import org.apache.camel.component.mail.Mailbox.Protocol;
+import org.apache.camel.component.mock.MockEndpoint;
+import org.apache.camel.test.junit6.CamelTestSupport;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+/**
+ * Unit test for Camel attachments and Mail attachments.
+ */
+public class MailAttachmentFileNameQuotingTest extends CamelTestSupport {
+ private static final String HOSTILE_NAME = "report.jpeg;
boundary=--injected";
+
+ private static final MailboxUser james =
Mailbox.getOrCreateUser("MailAttachmentFileNameQuotingTest-james", "secret");
+
+ @Test
+ public void hostileAttachmentNameIsQuotedInTheContentTypeHeader() throws
Exception {
+ // clear mailbox
+ Mailbox.clearAll();
+
+ // create an exchange with a normal body and attachment to be produced
as email
+ Endpoint endpoint =
context.getEndpoint(james.uriPrefix(Protocol.smtp));
+
+ // create the exchange with the mail message that is multipart with a
file and a Hello World text/plain message.
+ Exchange exchange = endpoint.createExchange();
+ AttachmentMessage in = exchange.getIn(AttachmentMessage.class);
+ in.setBody("Hello World");
+ in.addAttachment(HOSTILE_NAME, new DataHandler(new
FileDataSource("src/test/data/logo.jpeg")));
+
+ // create a producer that can produce the exchange (= send the mail)
+ Producer producer = endpoint.createProducer();
+ // start the producer
+ producer.start();
+ // and let it go (processes the exchange by sending the email)
+ producer.process(exchange);
+
+ MockEndpoint mock = getMockEndpoint("mock:result");
+ mock.expectedMessageCount(1);
+ mock.assertIsSatisfied();
+ Exchange out = mock.assertExchangeReceived(0);
+
+ // plain text
+ assertEquals("Hello World", out.getIn().getBody(String.class));
+
+ // attachment
+ Map<String, DataHandler> attachments =
out.getIn(AttachmentMessage.class).getAttachments();
+ assertNotNull(attachments, "Should have attachments");
+ assertEquals(1, attachments.size());
+
+ DataHandler handler = attachments.values().iterator().next();
+ assertNotNull(handler, "The attachment should be there");
+
+ // The file name is relayed from the incoming message, so it must be
emitted as a quoted
+ // parameter value. Concatenating it lets a name containing a
semicolon add parameters of its
+ // own to the header - here a second boundary declaration.
+ // Parse it rather than string-match: the hostile text appearing
anywhere in the header is fine,
+ // what matters is whether it is inside the quoted name or has become
a parameter of its own.
+ ContentType contentType = new ContentType(handler.getContentType());
+ assertNull(contentType.getParameter("boundary"),
+ "Attachment name must not introduce a parameter: " +
handler.getContentType());
+ assertEquals(HOSTILE_NAME, contentType.getParameter("name"),
+ "The whole name must survive as a single parameter value");
+
+ producer.stop();
+ }
+
+ @Override
+ protected RouteBuilder createRouteBuilder() {
+ return new RouteBuilder() {
+ public void configure() {
+ MailComponent mail = getContext().getComponent("smtp",
MailComponent.class);
+ mail.setContentTypeResolver(new ContentTypeResolver() {
+ public String resolveContentType(String fileName) {
+ return "image/jpeg";
+ }
+ });
+
+ from(james.uriPrefix(Protocol.pop3) +
"&initialDelay=100&delay=100")
+ .convertBodyTo(String.class)
+ .to("mock:result");
+ }
+ };
+ }
+}
diff --git
a/components/camel-mail/src/test/java/org/apache/camel/dataformat/mime/multipart/MimeMultipartDataFormatTest.java
b/components/camel-mail/src/test/java/org/apache/camel/dataformat/mime/multipart/MimeMultipartDataFormatTest.java
index d81cebb3fd6c..72500ed71172 100644
---
a/components/camel-mail/src/test/java/org/apache/camel/dataformat/mime/multipart/MimeMultipartDataFormatTest.java
+++
b/components/camel-mail/src/test/java/org/apache/camel/dataformat/mime/multipart/MimeMultipartDataFormatTest.java
@@ -428,6 +428,31 @@ public class MimeMultipartDataFormatTest extends
CamelTestSupport {
assertNull(out.getMessage().getHeader("CAMELBaz"));
}
+ @Test
+ void unmarshalAttachmentNameIsReducedToALeafName() {
+ // the attachment file name is chosen by the sender of the message
being unmarshalled
+ in.setBody(new
File("src/test/resources/multipart-traversal-name.txt"));
+ Exchange out = template.send("direct:unmarshalonlyinlineheaders",
exchange);
+
+ AttachmentMessage am = out.getMessage(AttachmentMessage.class);
+ assertThat(am.getAttachmentNames())
+ .contains("evil.sh")
+ .doesNotContain("../../evil.sh");
+ }
+
+ @Test
+ void unmarshalContentIdKeyKeepsSlashesBecauseItIsNotAPath() {
+ // a Content-ID local part may legally contain '/' (RFC 5322 atext);
it is an identifier, not a sender-chosen
+ // file name, so it must not be reduced to a leaf name the way a file
name is
+ in.setBody(new
File("src/test/resources/multipart-contentid-with-slash.txt"));
+ Exchange out = template.send("direct:unmarshalonlyinlineheaders",
exchange);
+
+ AttachmentMessage am = out.getMessage(AttachmentMessage.class);
+ assertThat(am.getAttachmentNames())
+ .contains("a/[email protected]")
+ .doesNotContain("[email protected]");
+ }
+
@Test
void unmarshalInlineHeadersFiltersMailSessionPropertyHeaders() {
// MailHeaderFilterStrategy adds the mail.smtp. / mail.smtps. prefixes
to the inbound filter
diff --git
a/components/camel-mail/src/test/resources/multipart-contentid-with-slash.txt
b/components/camel-mail/src/test/resources/multipart-contentid-with-slash.txt
new file mode 100644
index 000000000000..b8def3a96ebc
--- /dev/null
+++
b/components/camel-mail/src/test/resources/multipart-contentid-with-slash.txt
@@ -0,0 +1,13 @@
+MIME-Version: 1.0
+Content-Type: Multipart/Related; boundary=example-1
+
+--example-1
+Content-Type: text/plain
+
+the body
+--example-1
+Content-Type: text/plain
+Content-ID: <a/[email protected]>
+
+related part whose Content-ID local part contains a slash
+--example-1--
diff --git
a/components/camel-mail/src/test/resources/multipart-traversal-name.txt
b/components/camel-mail/src/test/resources/multipart-traversal-name.txt
new file mode 100644
index 000000000000..8ceaa60f7801
--- /dev/null
+++ b/components/camel-mail/src/test/resources/multipart-traversal-name.txt
@@ -0,0 +1,21 @@
+MIME-Version: 1.0
+Content-Type: Multipart/Related; boundary=example-1;
start="<[email protected]>"; type="Application/X-FixedRecord";
start-info="-o ps"
+
+--example-1
+Content-Type: Application/X-FixedRecord
+Content-ID: <[email protected]>
+
+25
+10
+34
+10
+25
+21
+26
+10
+--example-1
+Content-Type: Application/octet-stream
+Content-Disposition: attachment; filename="../../evil.sh"
+
+payload
+--example-1--
diff --git
a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
index 1e20dc11dd8e..e4ec6f31e52b 100644
--- a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
+++ b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
@@ -1094,6 +1094,13 @@ Routes that relied on `+mail.smtp.*+` or
`+mail.smtps.*+` headers arriving on th
unmarshalled MIME message must set those values explicitly on the route
instead. Ordinary
application headers are unaffected.
+`MailBinding` now parses the content type returned by a custom
`ContentTypeResolver` before adding the
+attachment file name as a quoted parameter, instead of concatenating the file
name into the header - closing
+a header-injection vector via a crafted file name. As a side effect, a
`ContentTypeResolver` that returns an
+unparsable content type now fails fast with a `ParseException` at marshal
time, where before the invalid
+value was written into the `Content-Type` header as-is. Ensure a custom
`ContentTypeResolver` returns a valid
+MIME type.
+
=== camel-netty - object codecs apply a deserialization filter by default
The `ObjectDecoder` and `DatagramPacketObjectDecoder` codecs (used when a
route configures Netty
