This is an automated email from the ASF dual-hosted git repository.
reta pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/main by this push:
new 6d0d519ea91 strip path components from content-disposition filename
(#3572)
6d0d519ea91 is described below
commit 6d0d519ea9164eb06aecad1538eab09454338b99
Author: Javid Khan <[email protected]>
AuthorDate: Sun Oct 18 20:32:15 2026 +0530
strip path components from content-disposition filename (#3572)
* strip path components from content-disposition filename
* sanitise inbound attachment filename at the consumer via
FileUtils.safeFileName
* neutralise unsafe filename chars and key file parts off the raw filename
replace NUL and ':' with '_' in FileUtils.safeFileName rather than dropping
an
otherwise valid name like Meeting 10:30.pdf, and decide whether an
EntityPart is
a file from the raw cd.getFilename() so a value such as reports/ or ..
stays a
file part while safeFileName only cleans the reported name. adds a jaxrs
EntityPart test covering the traversal, plain and trailing-separator cases
and a
javadoc note on ContentDisposition.getFilename().
* keep an empty filename a form field, not a file part
an empty filename= (what browsers send for an unfilled HTML file input) was
counting as a file part once the file check moved to the raw value, so it
went
down the file branch and lost its form-field name. decide on
!StringUtils.isEmpty(rawFileName) instead, including the
createFromAttachment
check, so filename= stays a form field while reports/ and .. still stay file
parts.
* add ContentDisposition.getFilenameSanitized and use it in the entity part
code
* keep the entity part filename raw for pass-through re-serialisation
---
.../org/apache/cxf/attachment/AttachmentUtil.java | 5 +-
.../apache/cxf/attachment/ContentDisposition.java | 18 ++++++
.../java/org/apache/cxf/helpers/FileUtils.java | 20 +++++++
.../apache/cxf/attachment/AttachmentUtilTest.java | 45 +++++++++++++++
.../cxf/attachment/ContentDispositionTest.java | 15 ++++-
.../java/org/apache/cxf/helpers/FileUtilsTest.java | 30 ++++++++++
.../cxf/jaxrs/provider/EntityPartProvider.java | 8 ++-
.../apache/cxf/jaxrs/utils/EntityPartUtils.java | 8 +--
.../cxf/jaxrs/provider/EntityPartProviderTest.java | 66 +++++++++++++++++++++-
9 files changed, 205 insertions(+), 10 deletions(-)
diff --git a/core/src/main/java/org/apache/cxf/attachment/AttachmentUtil.java
b/core/src/main/java/org/apache/cxf/attachment/AttachmentUtil.java
index 3ea312238e1..ee5ab49b88c 100644
--- a/core/src/main/java/org/apache/cxf/attachment/AttachmentUtil.java
+++ b/core/src/main/java/org/apache/cxf/attachment/AttachmentUtil.java
@@ -442,8 +442,9 @@ public final class AttachmentUtil {
headers.remove("Content-Transfer-Encoding");
}
DataSource source = new AttachmentDataSource(ct, ins);
- if (!StringUtils.isEmpty(fileName)) {
-
((AttachmentDataSource)source).setName(FileUtils.stripPath(fileName));
+ String safeFileName = FileUtils.safeFileName(fileName);
+ if (!StringUtils.isEmpty(safeFileName)) {
+ ((AttachmentDataSource)source).setName(safeFileName);
}
att.setDataHandler(new DataHandler(source));
return att;
diff --git
a/core/src/main/java/org/apache/cxf/attachment/ContentDisposition.java
b/core/src/main/java/org/apache/cxf/attachment/ContentDisposition.java
index 44a2d710270..ce9f72814f6 100644
--- a/core/src/main/java/org/apache/cxf/attachment/ContentDisposition.java
+++ b/core/src/main/java/org/apache/cxf/attachment/ContentDisposition.java
@@ -26,6 +26,8 @@ import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import org.apache.cxf.helpers.FileUtils;
+
public class ContentDisposition {
private static final String CD_HEADER_PARAMS_EXPRESSION =
"[\\w-]++( )?\\*?=( )?((\"[^\"]++\")|([^;]+))";
@@ -127,10 +129,26 @@ public class ContentDisposition {
return type;
}
+ /**
+ * Returns the raw filename as it appeared in the header. The value is
parsed
+ * straight from attacker-controlled input and is not sanitised here, so
it may
+ * still contain path separators or traversal sequences; callers building
a file
+ * from it should use {@link #getFilenameSanitized()} instead.
+ */
public String getFilename() {
return params.get(FILE_NAME);
}
+ /**
+ * Returns the filename reduced by {@link FileUtils#safeFileName(String)}
to a
+ * value that is safe to build a file from, or null if the header carried
no
+ * filename or nothing usable is left once it is sanitised (for example
".."
+ * or a name ending in a path separator).
+ */
+ public String getFilenameSanitized() {
+ return FileUtils.safeFileName(getFilename());
+ }
+
public String getParameter(String name) {
return params.get(name);
}
diff --git a/core/src/main/java/org/apache/cxf/helpers/FileUtils.java
b/core/src/main/java/org/apache/cxf/helpers/FileUtils.java
index 18a330da139..6795baf1b71 100644
--- a/core/src/main/java/org/apache/cxf/helpers/FileUtils.java
+++ b/core/src/main/java/org/apache/cxf/helpers/FileUtils.java
@@ -349,4 +349,24 @@ public final class FileUtils {
return name;
}
+ /**
+ * Reduces an untrusted file name, for example one parsed from an inbound
+ * Content-Disposition header, to a value that is safe to build a file
from.
+ * Any directory component is stripped (see {@link #stripPath(String)})
and a
+ * name that would still walk a caller doing {@code new File(dir, name)}
out of
+ * its directory - "." or "..", or one that strips down to nothing - is
rejected
+ * by returning null. A NUL or a ':' (which Windows can treat as a
drive-relative
+ * path) is neutralised to '_' rather than rejected, so an otherwise valid
name
+ * such as "Meeting 10:30.pdf" keeps a usable form. Callers should treat a
null
+ * result as the part having no usable file name.
+ */
+ public static String safeFileName(String name) {
+ String stripped = stripPath(name);
+ if (stripped == null || stripped.isEmpty()
+ || ".".equals(stripped) || "..".equals(stripped)) {
+ return null;
+ }
+ return stripped.replace('\0', '_').replace(':', '_');
+ }
+
}
diff --git
a/core/src/test/java/org/apache/cxf/attachment/AttachmentUtilTest.java
b/core/src/test/java/org/apache/cxf/attachment/AttachmentUtilTest.java
index 41c76ba274d..5637d077005 100644
--- a/core/src/test/java/org/apache/cxf/attachment/AttachmentUtilTest.java
+++ b/core/src/test/java/org/apache/cxf/attachment/AttachmentUtilTest.java
@@ -41,6 +41,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.endsWith;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
@@ -408,4 +409,48 @@ public class AttachmentUtilTest {
new ByteArrayInputStream("3".getBytes(StandardCharsets.UTF_8)),
headers, message);
return att.getDataHandler().getContentType();
}
+
+ @Test
+ public void testCreateAttachmentStripsPathFromFilename() throws
IOException {
+ assertEquals("passwd",
+ createAttachmentName("form-data; name=\"file\";
filename=\"../../../etc/passwd\""));
+ assertEquals("secret.txt",
+ createAttachmentName("form-data; name=\"file\";
filename=\"..\\..\\secret.txt\""));
+ }
+
+ @Test
+ public void testCreateAttachmentStripsExtendedFilenameTraversal() throws
IOException {
+ // CXF percent-decodes the filename* value, turning %2e%2e%2f back
into ../
+ assertEquals("passwd",
+ createAttachmentName("form-data; name=\"file\";
filename*=UTF-8''%2e%2e%2f%2e%2e%2fpasswd"));
+ }
+
+ @Test
+ public void testCreateAttachmentStripsCodepointEncodedTraversal() throws
IOException {
+ // &#NNNN; references are decoded too: ../ becomes
../
+ assertEquals("x",
+ createAttachmentName("form-data; name=\"file\";
filename=\"../x\""));
+ }
+
+ @Test
+ public void testCreateAttachmentDropsUnsafeFilename() throws IOException {
+ // ".." and a trailing separator leave nothing usable, so no name is
set
+ assertNull(createAttachmentName("form-data; name=\"file\";
filename=\"..\""));
+ assertNull(createAttachmentName("form-data; name=\"file\";
filename=\"reports/\""));
+ }
+
+ @Test
+ public void testCreateAttachmentNeutralisesNulInFilename() throws
IOException {
+ // a NUL is replaced rather than dropped, so the part keeps a usable
name
+ assertEquals("shell.jsp_.png",
+ createAttachmentName("form-data; name=\"file\";
filename=\"shell.jsp\u0000.png\""));
+ }
+
+ private static String createAttachmentName(String cd) throws IOException {
+ Map<String, List<String>> headers = new
TreeMap<>(String.CASE_INSENSITIVE_ORDER);
+ headers.put("Content-Disposition", Collections.singletonList(cd));
+ Attachment att = AttachmentUtil.createAttachment(
+ new ByteArrayInputStream("3".getBytes(StandardCharsets.UTF_8)),
headers, new MessageImpl());
+ return att.getDataHandler().getName();
+ }
}
diff --git
a/core/src/test/java/org/apache/cxf/attachment/ContentDispositionTest.java
b/core/src/test/java/org/apache/cxf/attachment/ContentDispositionTest.java
index 45f503dac19..9c104415723 100644
--- a/core/src/test/java/org/apache/cxf/attachment/ContentDispositionTest.java
+++ b/core/src/test/java/org/apache/cxf/attachment/ContentDispositionTest.java
@@ -45,4 +45,17 @@ public class ContentDispositionTest {
assertNull(cd.getFilename());
}
-}
\ No newline at end of file
+
+ @Test
+ public void testGetFilenameSanitized() {
+ ContentDisposition cd = new
ContentDisposition("attachment;filename=\"../../../etc/passwd\"");
+ assertEquals("../../../etc/passwd", cd.getFilename());
+ assertEquals("passwd", cd.getFilenameSanitized());
+
+ cd = new
ContentDisposition("attachment;filename*=UTF-8''%2e%2e%2f%2e%2e%2fpasswd");
+ assertEquals("passwd", cd.getFilenameSanitized());
+
+ assertNull(new
ContentDisposition("attachment;filename=\"..\"").getFilenameSanitized());
+ assertNull(new
ContentDisposition("attachment;name=\"field\"").getFilenameSanitized());
+ }
+}
diff --git a/core/src/test/java/org/apache/cxf/helpers/FileUtilsTest.java
b/core/src/test/java/org/apache/cxf/helpers/FileUtilsTest.java
index f816dd9c2c0..32dd2df7dbe 100644
--- a/core/src/test/java/org/apache/cxf/helpers/FileUtilsTest.java
+++ b/core/src/test/java/org/apache/cxf/helpers/FileUtilsTest.java
@@ -29,7 +29,9 @@ import java.util.Optional;
import org.junit.Test;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
public class FileUtilsTest {
@@ -78,4 +80,32 @@ public class FileUtilsTest {
assertTrue(foundFiles.containsAll(foundFiles2) &&
foundFiles2.containsAll(foundFiles));
}
+
+ @Test
+ public void testSafeFileNameKeepsFinalSegment() {
+ assertEquals("passwd", FileUtils.safeFileName("../../../etc/passwd"));
+ assertEquals("secret.txt",
FileUtils.safeFileName("..\\..\\secret.txt"));
+ assertEquals("report.pdf", FileUtils.safeFileName("report.pdf"));
+ }
+
+ @Test
+ public void testSafeFileNameRejectsTraversalAndEmpty() {
+ assertNull(FileUtils.safeFileName(".."));
+ assertNull(FileUtils.safeFileName("."));
+ assertNull(FileUtils.safeFileName("a/.."));
+ // a trailing separator strips down to an empty name, which would
resolve
+ // back to the upload directory itself
+ assertNull(FileUtils.safeFileName("x/"));
+ assertNull(FileUtils.safeFileName(""));
+ assertNull(FileUtils.safeFileName(null));
+ }
+
+ @Test
+ public void testSafeFileNameNeutralisesNulAndColon() {
+ // NUL and ':' are replaced rather than rejected, so an otherwise valid
+ // name keeps a usable form instead of being dropped altogether
+ assertEquals("shell.jsp_.png",
FileUtils.safeFileName("shell.jsp\u0000.png"));
+ assertEquals("C_evil.exe", FileUtils.safeFileName("C:evil.exe"));
+ assertEquals("Meeting 10_30.pdf", FileUtils.safeFileName("Meeting
10:30.pdf"));
+ }
}
diff --git
a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/EntityPartProvider.java
b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/EntityPartProvider.java
index e2be13b641b..80a623e52d2 100644
---
a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/EntityPartProvider.java
+++
b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/EntityPartProvider.java
@@ -169,9 +169,13 @@ public class EntityPartProvider extends
AbstractConfigurableProvider
final String cdName = cd == null ? null : cd.getParameter("name");
final String contentId = attachment.getContentId();
+ // the filename is carried through verbatim: EntityPartImpl passes
it straight
+ // back out when the part is re-serialised, so sanitising it here
would corrupt
+ // a part that is simply read and forwarded. Callers that build a
file from the
+ // name should reduce it through
ContentDisposition.getFilenameSanitized().
if (!StringUtils.isEmpty(fileName)) {
- return new EntityPartImpl(mc.getProviders(), contentId,
fileName, is, c, genericType,
- attachment.getHeaders(), attachment.getContentType());
+ return new EntityPartImpl(mc.getProviders(), contentId,
fileName, is, c,
+ genericType, attachment.getHeaders(),
attachment.getContentType());
} else {
final String name = StringUtils.isEmpty(cdName) ? contentId :
cdName.replace("\"", "").replace("'", "");
return new EntityPartImpl(mc.getProviders(), name, null, is,
c, genericType, attachment.getHeaders(),
diff --git
a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/EntityPartUtils.java
b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/EntityPartUtils.java
index 6eeb8a13f1b..5d33d8d3dd7 100644
---
a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/EntityPartUtils.java
+++
b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/EntityPartUtils.java
@@ -59,8 +59,8 @@ final class EntityPartUtils {
final String fileName = (cd != null) ? cd.getFilename() : null;
if (!StringUtils.isEmpty(fileName)) {
- return new EntityPartImpl(providers, key, fileName, value,
String.class, null,
- attachment.getHeaders(), attachment.getContentType());
+ return new EntityPartImpl(providers, key, fileName, value,
String.class,
+ null, attachment.getHeaders(),
attachment.getContentType());
} else {
return new EntityPartImpl(providers, key, null, value,
String.class, null, attachment.getHeaders(),
attachment.getContentType());
@@ -110,8 +110,8 @@ final class EntityPartUtils {
try {
final InputStream is = a.getDataHandler().getInputStream();
if (!StringUtils.isEmpty(fileName)) {
- return new EntityPartImpl(providers, name, fileName, is,
InputStream.class, null,
- a.getHeaders(), a.getContentType());
+ return new EntityPartImpl(providers, name, fileName, is,
InputStream.class,
+ null, a.getHeaders(), a.getContentType());
} else {
return new EntityPartImpl(providers, name, null, is,
InputStream.class, null,
a.getHeaders(), a.getContentType());
diff --git
a/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/provider/EntityPartProviderTest.java
b/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/provider/EntityPartProviderTest.java
index 098009366ef..af765c3d6f7 100644
---
a/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/provider/EntityPartProviderTest.java
+++
b/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/provider/EntityPartProviderTest.java
@@ -25,6 +25,7 @@ import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.nio.charset.StandardCharsets;
import java.util.List;
+import java.util.Optional;
import java.util.stream.IntStream;
import jakarta.ws.rs.core.EntityPart;
@@ -161,10 +162,73 @@ public class EntityPartProviderTest {
msg.put(Message.CONTENT_TYPE, "multipart/related");
assertThrows("Failure expected on too many attachment headers",
RuntimeException.class,
- () -> p.readFrom((Class<List<EntityPart>>) (Class<?>)
List.class,
+ () -> p.readFrom((Class<List<EntityPart>>) (Class<?>)
List.class,
EntityPart.class, new Annotation[]{},
MediaType.APPLICATION_OCTET_STREAM_TYPE,
new MetadataMap<String, String>(),
msg.getContent(InputStream.class)));
}
+
+ @Test
+ public void testFilenameIsCarriedThroughRaw() throws Exception {
+ // the EntityPart keeps the filename verbatim so a read-and-forwarded
part is not
+ // corrupted; sanitising is left to callers via
ContentDisposition.getFilenameSanitized()
+ final List<EntityPart> parts = readParts("form-data; name=\"file\";
filename=\"../../../etc/passwd\"");
+ assertEquals(1, parts.size());
+ assertEquals(Optional.of("../../../etc/passwd"),
parts.get(0).getFileName());
+ }
+
+ @Test
+ public void testPlainFilenamePassesThrough() throws Exception {
+ final List<EntityPart> parts = readParts("form-data; name=\"file\";
filename=\"report.pdf\"");
+ assertEquals(1, parts.size());
+ assertEquals(Optional.of("report.pdf"), parts.get(0).getFileName());
+ }
+
+ @Test
+ public void testTrailingSeparatorStaysAFilePart() throws Exception {
+ // a non-empty filename in the header makes this a file part (named by
its
+ // content id) rather than a plain form field, and the value is kept
raw
+ final List<EntityPart> parts = readParts("form-data; name=\"file\";
filename=\"reports/\"");
+ assertEquals(1, parts.size());
+ assertEquals(Optional.of("reports/"), parts.get(0).getFileName());
+ assertEquals("file-part", parts.get(0).getName());
+ }
+
+ @Test
+ public void testEmptyFilenameIsAFormField() throws Exception {
+ // browsers send filename="" for an empty HTML file input, so an empty
value
+ // stays a plain form field named after its part name rather than a
file part
+ final List<EntityPart> parts = readParts("form-data; name=\"file\";
filename=\"\"");
+ assertEquals(1, parts.size());
+ assertEquals(Optional.empty(), parts.get(0).getFileName());
+ assertEquals("file", parts.get(0).getName());
+ }
+
+ @SuppressWarnings("unchecked")
+ private List<EntityPart> readParts(final String contentDisposition) throws
Exception {
+ final Exchange exchange = new ExchangeImpl();
+ final EntityPartProvider p = new EntityPartProvider();
+
+ final String boundary = "----=_Part_34950_1098328613.1263781527359";
+ final String body = "--" + boundary + "\n"
+ + "Content-Type: application/octet-stream\n"
+ + "Content-Disposition: " + contentDisposition + "\n"
+ + "Content-Id: <file-part>\n"
+ + '\n'
+ + "some bytes\n"
+ + "--" + boundary + "--\n";
+
+ final Message msg = new MessageImpl();
+ msg.setExchange(exchange);
+ exchange.setInMessage(msg);
+ p.setMessageContext(new MessageContextImpl(msg));
+ p.setProviders(providers);
+
+ msg.setContent(InputStream.class, new
ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8)));
+ msg.put(Message.CONTENT_TYPE, "multipart/form-data; boundary=\"" +
boundary + "\"");
+
+ return p.readFrom((Class<List<EntityPart>>) (Class<?>) List.class,
EntityPart.class, new Annotation[]{},
+ MediaType.MULTIPART_FORM_DATA_TYPE, new MetadataMap<String,
String>(), msg.getContent(InputStream.class));
+ }
}
