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 fd120518ecbd CAMEL-24427: camel-servlet, camel-jetty - enforce
fileNameExtWhitelist on the submitted file name (#26189)
fd120518ecbd is described below
commit fd120518ecbd591b05853434225587d2bd76b2fd
Author: Andrea Cosentino <[email protected]>
AuthorDate: Tue Sep 8 15:24:55 2026 +0200
CAMEL-24427: camel-servlet, camel-jetty - enforce fileNameExtWhitelist on
the submitted file name (#26189)
camel-servlet's AttachmentHttpBinding checked the whitelist against
Part.getName(), which is the multipart field name and not the submitted file
name. A field named "file" carries no extension, so FileUtil.onlyExt
returned
null and no upload was ever rejected. It now checks
Part.getSubmittedFileName(), which is what camel-platform-http-vertx already
does.
The camel-jetty binding performed no whitelist check at all, although
fileNameExtWhitelist can be set on its HttpBinding. It now applies the same
check.
The camel-jetty binding also stored the attachment under the multipart field
name but looked it up again by the submitted file name, so the lookup only
succeeded when the two happened to be equal, and the resulting header was
named
by the client-supplied file name. The attachment is now looked up and
exposed
under the field name it is stored with, and only for parts that carry a file
name.
The whitelist is matched per comma-separated extension exactly rather than
with
a substring test, and the client-controlled file name is passed through
HttpHelper.sanitizeLog before being logged.
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
Co-authored-by: Claude Opus 4.8 <[email protected]>
---
.../component/jetty12/AttachmentHttpBinding.java | 59 ++++++++++--
.../MultiPartFormFileNameExtWhitelistTest.java | 101 +++++++++++++++++++++
.../component/servlet/AttachmentHttpBinding.java | 22 ++++-
.../MultipartUploadFileNameExtWhitelistTest.java | 91 +++++++++++++++++++
.../ROOT/pages/camel-4x-upgrade-guide-4_23.adoc | 26 ++++++
5 files changed, 285 insertions(+), 14 deletions(-)
diff --git
a/components/camel-jetty/src/main/java/org/apache/camel/component/jetty12/AttachmentHttpBinding.java
b/components/camel-jetty/src/main/java/org/apache/camel/component/jetty12/AttachmentHttpBinding.java
index fd15b719c2cc..872a6615ff1d 100644
---
a/components/camel-jetty/src/main/java/org/apache/camel/component/jetty12/AttachmentHttpBinding.java
+++
b/components/camel-jetty/src/main/java/org/apache/camel/component/jetty12/AttachmentHttpBinding.java
@@ -21,6 +21,7 @@ import java.io.InputStream;
import java.io.OutputStream;
import java.util.Collection;
import java.util.Enumeration;
+import java.util.Locale;
import java.util.Map;
import jakarta.activation.DataHandler;
@@ -37,6 +38,7 @@ import org.apache.camel.attachment.DefaultAttachmentMessage;
import org.apache.camel.component.jetty.MultiPartFilter;
import org.apache.camel.http.common.DefaultHttpBinding;
import org.apache.camel.http.common.HttpHelper;
+import org.apache.camel.util.FileUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -59,6 +61,16 @@ final class AttachmentHttpBinding extends DefaultHttpBinding
{
try {
parts = request.getParts();
for (Part part : parts) {
+ // the whitelist accepts file name extensions, so it must
be checked against the submitted
+ // file name and not against Part.getName(), which is the
multipart field name
+ String fileName = part.getSubmittedFileName();
+ if (!isFileNameAccepted(fileName)) {
+ LOG.debug(
+ "Cannot add file as attachment: {} because the
file is not accepted according to fileNameExtWhitelist: {}",
+ HttpHelper.sanitizeLog(fileName),
getFileNameExtWhitelist());
+ continue;
+ }
+
DataSource ds = new PartDataSource(part);
Attachment attachment = new DefaultAttachment(ds);
for (String headerName : part.getHeaderNames()) {
@@ -67,16 +79,21 @@ final class AttachmentHttpBinding extends
DefaultHttpBinding {
}
}
AttachmentMessage am = new
DefaultAttachmentMessage(message);
- am.addAttachmentObject(part.getName(), attachment);
- String name = part.getSubmittedFileName();
- Object value = am.getAttachment(name);
- Map<String, Object> headers = message.getHeaders();
- if (getHeaderFilterStrategy() != null
- &&
!getHeaderFilterStrategy().applyFilterToExternalHeaders(name, value,
message.getExchange())
- && name != null) {
- HttpHelper.appendHeader(headers, name, value);
+ String name = part.getName();
+ am.addAttachmentObject(name, attachment);
+ // a file part is also exposed as a header carrying the
DataHandler. The attachment is keyed on
+ // the multipart field name, so the header must be looked
up and named by that same key and not
+ // by the client supplied file name. A plain form field
carries no file name and is mapped by
+ // populateRequestParameters instead, so it is left alone
here.
+ if (fileName != null && name != null) {
+ Object value = am.getAttachment(name);
+ Map<String, Object> headers = message.getHeaders();
+ if (getHeaderFilterStrategy() != null
+ &&
!getHeaderFilterStrategy().applyFilterToExternalHeaders(name, value,
+ message.getExchange())) {
+ HttpHelper.appendHeader(headers, name, value);
+ }
}
-
}
} catch (Exception e) {
throw new RuntimeCamelException("Cannot populate attachments",
e);
@@ -84,6 +101,30 @@ final class AttachmentHttpBinding extends
DefaultHttpBinding {
}
}
+ private boolean isFileNameAccepted(String fileName) {
+ String whitelist = getFileNameExtWhitelist();
+ if (whitelist == null) {
+ return true;
+ }
+ String ext = FileUtil.onlyExt(fileName);
+ if (ext == null) {
+ return true;
+ }
+ ext = ext.toLowerCase(Locale.US);
+ whitelist = whitelist.toLowerCase(Locale.US);
+ if (whitelist.equals("*")) {
+ return true;
+ }
+ // compare against each comma-separated extension exactly, not as a
substring: a whitelist of "txt"
+ // must not accept an upload named "evil.x" just because
"txt".contains("x")
+ for (String allowed : whitelist.split(",")) {
+ if (allowed.trim().equals(ext)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
@Override
protected void populateRequestParameters(HttpServletRequest request,
Message message) {
// we populate the http request parameters without checking the request
diff --git
a/components/camel-jetty/src/test/java/org/apache/camel/component/jetty/MultiPartFormFileNameExtWhitelistTest.java
b/components/camel-jetty/src/test/java/org/apache/camel/component/jetty/MultiPartFormFileNameExtWhitelistTest.java
new file mode 100644
index 000000000000..a8ae1d0a55c5
--- /dev/null
+++
b/components/camel-jetty/src/test/java/org/apache/camel/component/jetty/MultiPartFormFileNameExtWhitelistTest.java
@@ -0,0 +1,101 @@
+/*
+ * 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.jetty;
+
+import java.io.File;
+
+import jakarta.activation.DataHandler;
+
+import org.apache.camel.Exchange;
+import org.apache.camel.attachment.AttachmentMessage;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.hc.client5.http.entity.mime.MultipartEntityBuilder;
+import org.apache.hc.core5.http.ContentType;
+import org.apache.hc.core5.http.HttpEntity;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * The uploaded part carries the multipart field name {@code upload} and the
submitted file name
+ * {@code log4j2.properties}. The two differ, which is what tells the field
name apart from the file name in both the
+ * whitelist check and the header that exposes the attachment.
+ */
+public class MultiPartFormFileNameExtWhitelistTest extends BaseJettyTest {
+
+ private static final String FIELD_NAME = "upload";
+ private static final String FILE_NAME = "log4j2.properties";
+
+ @Test
+ public void testUploadAcceptedWhenExtensionIsWhitelisted() {
+ // the attachment is keyed on the field name, and so is the header
that exposes it. The client supplied
+ // file name must not become a header name.
+
assertThat(upload("/allowed")).isEqualTo("attachment=true,headerIsAttachment=true,fileNameHeader=false");
+ }
+
+ @Test
+ public void testUploadRejectedWhenExtensionIsNotWhitelisted() {
+
assertThat(upload("/rejected")).isEqualTo("attachment=false,headerIsAttachment=false,fileNameHeader=false");
+ }
+
+ @Test
+ public void
testUploadRejectedWhenExtensionIsOnlyASubstringOfTheWhitelist() {
+ // "propertiesx".contains("properties") is true, but exact matching
must reject the upload
+
assertThat(upload("/substring")).isEqualTo("attachment=false,headerIsAttachment=false,fileNameHeader=false");
+ }
+
+ private String upload(String path) {
+ File file = new File("src/test/resources/log4j2.properties");
+ HttpEntity entity = MultipartEntityBuilder.create()
+ .addBinaryBody(FIELD_NAME, file,
ContentType.APPLICATION_OCTET_STREAM, FILE_NAME)
+ .build();
+ return template.requestBody("http://localhost:"; + getPort() + path,
entity, String.class);
+ }
+
+ @Override
+ protected RouteBuilder createRouteBuilder() {
+ return new RouteBuilder() {
+ public void configure() {
+ getContext().getGlobalOptions().put("CamelJettyTempDir",
"target");
+
+ from(whitelisted("/allowed", "properties"))
+
.process(MultiPartFormFileNameExtWhitelistTest::reportAttachment);
+ from(whitelisted("/rejected", "pdf"))
+
.process(MultiPartFormFileNameExtWhitelistTest::reportAttachment);
+ from(whitelisted("/substring", "propertiesx"))
+
.process(MultiPartFormFileNameExtWhitelistTest::reportAttachment);
+ }
+
+ private JettyHttpEndpoint whitelisted(String path, String
whitelist) {
+ // the whitelist is not exposed as a jetty endpoint option, it
is configured on the binding
+ JettyHttpEndpoint endpoint = getContext().getEndpoint(
+ "jetty://http://localhost:"; + getPort() + path,
JettyHttpEndpoint.class);
+ endpoint.getHttpBinding().setFileNameExtWhitelist(whitelist);
+ return endpoint;
+ }
+ };
+ }
+
+ private static void reportAttachment(Exchange exchange) {
+ AttachmentMessage in = exchange.getIn(AttachmentMessage.class);
+ DataHandler attachment = in.getAttachment(FIELD_NAME);
+ Object headerByField = in.getHeader(FIELD_NAME);
+ exchange.getMessage().setBody("attachment=" + (attachment != null)
+ + ",headerIsAttachment=" + (attachment
!= null && headerByField == attachment)
+ + ",fileNameHeader=" +
(in.getHeader(FILE_NAME) != null));
+ }
+}
diff --git
a/components/camel-servlet/src/main/java/org/apache/camel/component/servlet/AttachmentHttpBinding.java
b/components/camel-servlet/src/main/java/org/apache/camel/component/servlet/AttachmentHttpBinding.java
index 0495a2cb9ab7..aeb78820d1b2 100644
---
a/components/camel-servlet/src/main/java/org/apache/camel/component/servlet/AttachmentHttpBinding.java
+++
b/components/camel-servlet/src/main/java/org/apache/camel/component/servlet/AttachmentHttpBinding.java
@@ -33,6 +33,7 @@ import org.apache.camel.attachment.AttachmentMessage;
import org.apache.camel.attachment.DefaultAttachment;
import org.apache.camel.attachment.DefaultAttachmentMessage;
import org.apache.camel.http.common.DefaultHttpBinding;
+import org.apache.camel.http.common.HttpHelper;
import org.apache.camel.util.FileUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -54,7 +55,9 @@ public final class AttachmentHttpBinding extends
DefaultHttpBinding {
try {
Collection<Part> parts = request.getParts();
for (Part part : parts) {
- String fileName = part.getName();
+ // the whitelist accepts file name extensions, so it must be
checked against the submitted file
+ // name and not against Part.getName(), which is the multipart
field name
+ String fileName = part.getSubmittedFileName();
// is the file name accepted
boolean accepted = true;
if (getFileNameExtWhitelist() != null) {
@@ -62,9 +65,7 @@ public final class AttachmentHttpBinding extends
DefaultHttpBinding {
if (ext != null) {
ext = ext.toLowerCase(Locale.US);
String whiteList =
getFileNameExtWhitelist().toLowerCase(Locale.US);
- if (!whiteList.equals("*") &&
!whiteList.contains(ext)) {
- accepted = false;
- }
+ accepted = whiteList.equals("*") ||
isExtWhitelisted(whiteList, ext);
}
}
@@ -81,7 +82,7 @@ public final class AttachmentHttpBinding extends
DefaultHttpBinding {
} else {
LOG.debug(
"Cannot add file as attachment: {} because the
file is not accepted according to fileNameExtWhitelist: {}",
- fileName, getFileNameExtWhitelist());
+ HttpHelper.sanitizeLog(fileName),
getFileNameExtWhitelist());
}
}
} catch (Exception e) {
@@ -89,6 +90,17 @@ public final class AttachmentHttpBinding extends
DefaultHttpBinding {
}
}
+ // compare against each comma-separated extension exactly, not as a
substring: a whitelist of "txt"
+ // must not accept an upload named "evil.x" just because
"txt".contains("x")
+ private static boolean isExtWhitelisted(String whitelist, String ext) {
+ for (String allowed : whitelist.split(",")) {
+ if (allowed.trim().equals(ext)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
public final class PartDataSource implements DataSource {
private final Part part;
diff --git
a/components/camel-servlet/src/test/java/org/apache/camel/component/servlet/MultipartUploadFileNameExtWhitelistTest.java
b/components/camel-servlet/src/test/java/org/apache/camel/component/servlet/MultipartUploadFileNameExtWhitelistTest.java
new file mode 100644
index 000000000000..17131da4a349
--- /dev/null
+++
b/components/camel-servlet/src/test/java/org/apache/camel/component/servlet/MultipartUploadFileNameExtWhitelistTest.java
@@ -0,0 +1,91 @@
+/*
+ * 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.servlet;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+import jakarta.servlet.MultipartConfigElement;
+
+import io.undertow.servlet.api.DeploymentInfo;
+import org.apache.camel.Exchange;
+import org.apache.camel.RoutesBuilder;
+import org.apache.camel.attachment.AttachmentMessage;
+import org.apache.camel.builder.RouteBuilder;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * The test harness posts a part whose multipart field name is {@code file}
and whose submitted file name is
+ * {@code test.txt}. The field name carries no extension, so checking the
whitelist against it never rejects anything.
+ */
+public class MultipartUploadFileNameExtWhitelistTest extends
ServletCamelRouterTestSupport {
+
+ @Override
+ protected DeploymentInfo getDeploymentInfo() {
+ DeploymentInfo deploymentInfo = super.getDeploymentInfo();
+ deploymentInfo.setDefaultMultipartConfig(new
MultipartConfigElement(System.getProperty("java.io.tmpdir")));
+ return deploymentInfo;
+ }
+
+ @Test
+ void testUploadAcceptedWhenExtensionIsWhitelisted() throws IOException {
+ assertEquals("accepted", upload("allowed"));
+ }
+
+ @Test
+ void testUploadRejectedWhenExtensionIsNotWhitelisted() throws IOException {
+ assertEquals("no attachment", upload("rejected"));
+ }
+
+ @Test
+ void testUploadRejectedWhenExtensionIsOnlyASubstringOfTheWhitelist()
throws IOException {
+ // the whitelist "txtdoc" must not accept a "test.txt" upload just
because "txtdoc".contains("txt")
+ assertEquals("no attachment", upload("substring"));
+ }
+
+ private String upload(String path) throws IOException {
+ InputStream body =
context.getTypeConverter().convertTo(InputStream.class, "Hello World");
+ PostMethodWebRequest request = new PostMethodWebRequest(
+ contextUrl + "/services/" + path, body, "multipart/form-data;
boundary=----Boundary");
+ return query(request).getText();
+ }
+
+ @Override
+ protected RoutesBuilder createRouteBuilder() {
+ return new RouteBuilder() {
+ @Override
+ public void configure() {
+
from("servlet:allowed?attachmentMultipartBinding=true&fileNameExtWhitelist=txt")
+
.process(MultipartUploadFileNameExtWhitelistTest::reportAttachment);
+
+
from("servlet:rejected?attachmentMultipartBinding=true&fileNameExtWhitelist=pdf")
+
.process(MultipartUploadFileNameExtWhitelistTest::reportAttachment);
+
+
from("servlet:substring?attachmentMultipartBinding=true&fileNameExtWhitelist=txtdoc")
+
.process(MultipartUploadFileNameExtWhitelistTest::reportAttachment);
+ }
+ };
+ }
+
+ private static void reportAttachment(Exchange exchange) {
+ AttachmentMessage message =
exchange.getMessage(AttachmentMessage.class);
+ boolean present = message.getAttachment("file") != null;
+ exchange.getMessage().setBody(present ? "accepted" : "no attachment");
+ }
+}
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 5a9f1abf0e83..fb25b895e6e9 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
@@ -1753,3 +1753,29 @@ custom `headerFilterStrategy` is used as-is and is
unaffected.
Routes that relied on one of those headers reaching the wire must set it
through the endpoint
configuration or supply a `headerFilterStrategy` that permits it.
+
+=== camel-servlet, camel-jetty - the multipart upload whitelist is enforced
against the submitted file name
+
+`fileNameExtWhitelist` accepts file name extensions, but `camel-servlet`'s
`AttachmentHttpBinding`
+checked it against `Part.getName()`, which is the multipart *field* name
rather than the submitted
+file name. A field named `file` carries no extension, so the check found
nothing to compare and every
+upload was accepted. The option is now checked against
`Part.getSubmittedFileName()`, which is what
+`camel-platform-http-vertx` already does.
+
+A `camel-servlet` consumer that sets `fileNameExtWhitelist` together with
`attachmentMultipartBinding=true`
+therefore starts rejecting uploads whose file extension is not listed, which
is what the option always
+advertised. Uploads with no file name, such as plain form fields, are
unaffected, and a route that does
+not set the option is unaffected. Review the configured extension list before
upgrading.
+
+The `camel-jetty` binding performed no whitelist check at all, although
`fileNameExtWhitelist` can be
+set on its `HttpBinding`. It now applies the same check.
+
+The `camel-jetty` binding also stored the attachment under the multipart field
name but looked it up
+again by the submitted file name, and passed that file name to
`HttpHelper.appendHeader`. The lookup
+therefore only succeeded when the two happened to be equal, and when it did
the header was named by
+the client-supplied file name. The attachment is now looked up and exposed
under the field name it is
+stored with, and only for parts that carry a file name — a plain form field is
mapped by
+`populateRequestParameters` as before. Because the old lookup by file name
returned `null` whenever the
+two names differed, that header never carried a usable `DataHandler` in the
first place; only when the
+names happened to be equal did it resolve, and then the name is unchanged. A
route that expected the
+attachment header under the uploaded file name should read it under the
multipart field name.
