This is an automated email from the ASF dual-hosted git repository. gnodet pushed a commit to branch backport/26189-to-camel-4.18.x in repository https://gitbox.apache.org/repos/asf/camel.git
commit 05185b35b55a52f3086e5daec85a2b3a90aecc8a Author: Guillaume Nodet <[email protected]> AuthorDate: Fri Sep 11 08:57:09 2026 +0000 [backport camel-4.18.x] CAMEL-24427: camel-servlet, camel-jetty - enforce fileNameExtWhitelist on the submitted file name --- .../component/jetty12/AttachmentHttpBinding.java | 59 ++++++++++-- .../MultiPartFormFileNameExtWhitelistTest.java | 101 +++++++++++++++++++++ .../component/servlet/AttachmentHttpBinding.java | 22 ++++- .../MultipartUploadFileNameExtWhitelistTest.java | 91 +++++++++++++++++++ .../ROOT/pages/camel-4x-upgrade-guide-4_18.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_18.adoc b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_18.adoc index c8c83b5f4366..e8c2a23e4080 100644 --- a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_18.adoc +++ b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_18.adoc @@ -1707,3 +1707,29 @@ Routes that set `useRecovery=false` are unaffected. Routes that left recovery en default, stop seeing in-progress aggregations re-delivered, and start seeing genuine recovery. The cache now also holds one entry per completed and not yet confirmed exchange; those entries are removed on confirmation. + +=== 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.
