diff --git a/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java b/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
index 038cef4bc..63afbdf22 100644
--- a/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
+++ b/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
@@ -35,6 +35,7 @@ import org.apache.logging.log4j.Logger;
import org.apache.struts2.StrutsConstants;
import org.apache.struts2.dispatcher.LocalizedMessage;
+import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Path;
@@ -44,6 +45,7 @@ import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.UUID;
import static org.apache.commons.lang3.StringUtils.normalizeSpace;
@@ -406,6 +408,22 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
return values.toArray(new String[0]);
}
+ /**
+ * Creates a secure temporary file in the specified directory using UUID-based naming.
+ * This method ensures files are created in a controlled location rather than the
+ * system temporary directory, reducing security risks.
+ *
+ * @param fileName the original filename for logging purposes
+ * @param location the directory where the temporary file should be created
+ * @return a new temporary file in the specified location
+ */
+ protected File createTemporaryFile(String fileName, Path location) {
+ String uid = UUID.randomUUID().toString().replace("-", "_");
+ File file = location.resolve("upload_" + uid + ".tmp").toFile();
+ LOG.debug("Creating temporary file: {} (originally: {})", file.getName(), fileName);
+ return file;
+ }
+
/* (non-Javadoc)
* @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#cleanUp()
*/
diff --git a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
index a8880d47a..23d93d585 100644
--- a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
+++ b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
@@ -103,7 +103,7 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
* @param saveDir the directory where uploaded files will be stored
* @throws IOException if an error occurs during upload processing
* @see #processNormalFormField(DiskFileItem, Charset)
- * @see #processFileField(DiskFileItem)
+ * @see #processFileField(DiskFileItem, String)
*/
@Override
protected void processUpload(HttpServletRequest request, String saveDir) throws IOException {
@@ -126,7 +126,7 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
} else {
// Process file upload fields
LOG.debug(() -> "Processing a file: " + normalizeSpace(item.getFieldName()));
- processFileField(item);
+ processFileField(item, saveDir);
}
}
}
@@ -193,7 +193,7 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
* @param item the disk file item representing the uploaded file
* @see #cleanUpTemporaryFiles()
*/
- protected void processFileField(DiskFileItem item) {
+ protected void processFileField(DiskFileItem item, String saveDir) {
// Skip file uploads that don't have a file name - meaning that no file was selected.
if (item.getName() == null || item.getName().trim().isEmpty()) {
LOG.debug(() -> "No file has been uploaded for the field: " + normalizeSpace(item.getFieldName()));
@@ -211,7 +211,7 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
if (item.isInMemory()) {
LOG.debug("Creating temporary file representing in-memory uploaded item: {}", normalizeSpace(item.getFieldName()));
try {
- File tempFile = File.createTempFile("struts_upload_", "_" + item.getName());
+ File tempFile = createTemporaryFile(item.getName(), Path.of(saveDir));
// Track the temporary file for explicit cleanup
temporaryFiles.add(tempFile);
@@ -299,7 +299,7 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
*
* This method deletes all temporary files that were created when
* processing in-memory uploads. These files are created in
- * {@link #processFileField(DiskFileItem)} when an uploaded file is
+ * {@link #processFileField(DiskFileItem, String)} when an uploaded file is
* stored in memory and needs to be written to disk.
*
* The cleanup process:
diff --git a/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequestTest.java b/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequestTest.java
index 83d6cd860..2ef0782ea 100644
--- a/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequestTest.java
+++ b/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequestTest.java
@@ -26,6 +26,8 @@ import java.io.File;
import java.io.IOException;
import java.lang.reflect.Field;
import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.util.List;
import static org.apache.commons.lang3.StringUtils.normalizeSpace;
@@ -112,7 +114,7 @@ public class JakartaMultiPartRequestTest extends AbstractMultiPartRequestTest {
// Create a custom implementation that simulates temp file creation failure
class FaultyJakartaMultiPartRequest extends JakartaMultiPartRequest {
@Override
- protected void processFileField(DiskFileItem item) {
+ protected void processFileField(DiskFileItem item, String saveDir) {
// Simulate in-memory upload that fails to create temp file
if (item.isInMemory()) {
try {
@@ -127,7 +129,7 @@ public class JakartaMultiPartRequestTest extends AbstractMultiPartRequestTest {
}
}
} else {
- super.processFileField(item);
+ super.processFileField(item, saveDir);
}
}
}
@@ -219,4 +221,64 @@ public class JakartaMultiPartRequestTest extends AbstractMultiPartRequestTest {
assertThat(multiPart.parameters).isEmpty();
}
+ @Test
+ public void temporaryFilesCreatedInSaveDirectory() throws IOException, NoSuchFieldException, IllegalAccessException {
+ // Test that temporary files for in-memory uploads are created in the saveDir, not system temp
+ String content = formFile("file1", "test1.csv", "small,content") +
+ endline + "--" + boundary + "--";
+
+ mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
+
+ // when
+ multiPart.parse(mockRequest, tempDir);
+
+ // Access private field to get temporary files
+ Field tempFilesField = JakartaMultiPartRequest.class.getDeclaredField("temporaryFiles");
+ tempFilesField.setAccessible(true);
+ @SuppressWarnings("unchecked")
+ List temporaryFiles = (List) tempFilesField.get(multiPart);
+
+ // then - verify temporary files are created in saveDir
+ assertThat(temporaryFiles).isNotEmpty();
+ for (File tempFile : temporaryFiles) {
+ // Verify the temporary file is in the saveDir, not system temp
+ assertThat(tempFile.getParent()).isEqualTo(tempDir);
+ assertThat(tempFile.getName()).startsWith("upload_");
+ assertThat(tempFile.getName()).endsWith(".tmp");
+ assertThat(tempFile).exists();
+ }
+ }
+
+ @Test
+ public void secureTemporaryFileNaming() throws IOException, NoSuchFieldException, IllegalAccessException {
+ // Test that temporary files use UUID-based naming for security
+ String content = formFile("file1", "malicious../../../etc/passwd", "content") +
+ endline + "--" + boundary + "--";
+
+ mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
+
+ // when
+ multiPart.parse(mockRequest, tempDir);
+
+ // Access private field to get temporary files
+ Field tempFilesField = JakartaMultiPartRequest.class.getDeclaredField("temporaryFiles");
+ tempFilesField.setAccessible(true);
+ @SuppressWarnings("unchecked")
+ List temporaryFiles = (List) tempFilesField.get(multiPart);
+
+ // then - verify secure naming prevents directory traversal
+ assertThat(temporaryFiles).isNotEmpty();
+ for (File tempFile : temporaryFiles) {
+ // Verify the temporary file uses secure UUID naming
+ assertThat(tempFile.getName()).startsWith("upload_");
+ assertThat(tempFile.getName()).endsWith(".tmp");
+ // Verify it doesn't contain malicious path elements
+ assertThat(tempFile.getName()).doesNotContain("..");
+ assertThat(tempFile.getName()).doesNotContain("/");
+ assertThat(tempFile.getName()).doesNotContain("\\");
+ // Verify it's in the correct directory
+ assertThat(tempFile.getParent()).isEqualTo(tempDir);
+ }
+ }
+
}