Sanitize file upload data

wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManagerTest.java

@@ -0,0 +1,96 @@
+/*
+ * 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.wicket.markup.html.form.upload.resource;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.apache.wicket.markup.html.form.upload.FileUpload;
+import org.apache.wicket.util.file.File;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+class FolderUploadsFileManagerTest
+{
+	@TempDir
+	Path tempDir;
+
+	@Test
+	void getFileRejectsTraversalInUploadFieldId()
+	{
+		FolderUploadsFileManager manager = new FolderUploadsFileManager(new File(tempDir.toFile()));
+
+		assertThrows(SecurityException.class, () -> manager.getFile("../escaped", "safe.txt"));
+	}
+
+	@Test
+	void getFileRejectsTraversalInClientFileName()
+	{
+		FolderUploadsFileManager manager = new FolderUploadsFileManager(new File(tempDir.toFile()));
+
+		assertThrows(SecurityException.class, () -> manager.getFile("uploadField", "../../etc/passwd"));
+	}
+
+	@Test
+	void saveRejectsTraversalInUploadFieldId() throws IOException
+	{
+		FolderUploadsFileManager manager = new FolderUploadsFileManager(new File(tempDir.toFile()));
+		FileUpload fileUpload = mock(FileUpload.class);
+		when(fileUpload.getClientFileName()).thenReturn("safe.txt");
+		when(fileUpload.getInputStream())
+			.thenReturn(new ByteArrayInputStream("content".getBytes(StandardCharsets.UTF_8)));
+
+		assertThrows(SecurityException.class, () -> manager.save(fileUpload, "../escaped"));
+	}
+
+	@Test
+	void saveRejectsTraversalInClientFileName() throws IOException
+	{
+		FolderUploadsFileManager manager = new FolderUploadsFileManager(new File(tempDir.toFile()));
+		FileUpload fileUpload = mock(FileUpload.class);
+		when(fileUpload.getClientFileName()).thenReturn("../../etc/passwd");
+		when(fileUpload.getInputStream())
+			.thenReturn(new ByteArrayInputStream("content".getBytes(StandardCharsets.UTF_8)));
+
+		assertThrows(SecurityException.class, () -> manager.save(fileUpload, "uploadField"));
+	}
+
+	@Test
+	void saveSucceedsWithValidUploadFieldIdAndClientFileName() throws IOException
+	{
+		FolderUploadsFileManager manager = new FolderUploadsFileManager(new File(tempDir.toFile()));
+		FileUpload fileUpload = mock(FileUpload.class);
+		when(fileUpload.getClientFileName()).thenReturn("safe.txt");
+		when(fileUpload.getInputStream())
+			.thenReturn(new ByteArrayInputStream("content".getBytes(StandardCharsets.UTF_8)));
+
+		manager.save(fileUpload, "uploadField");
+
+		Path savedFile = tempDir.resolve("uploadField").resolve("safe.txt");
+		assertTrue(Files.exists(savedFile));
+		assertEquals("content", Files.readString(savedFile, StandardCharsets.UTF_8));
+	}
+}

wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManager.java

@@ -18,7 +18,10 @@
 
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
+import java.nio.file.Path;
 import org.apache.wicket.WicketRuntimeException;
 import org.apache.wicket.markup.html.form.upload.FileUpload;
 import org.apache.wicket.util.file.File;
@@ -60,14 +63,62 @@ public File getFolder() {
         return folder;
     }
 
+    /**
+     * Returns the canonical path for a directory for use in path traversal checks.
+     * Uses {@code toRealPath()} when the directory exists; falls back to
+     * {@code toAbsolutePath().normalize()} only when the directory does not yet exist
+     * (e.g. an upload sub-folder that will be created during {@link #save}).
+     * Other {@link IOException} subtypes (e.g. permission errors) are intentionally
+     * re-thrown so they are not silently swallowed.
+     */
+    private static Path getPathForComparison(java.io.File dir) throws IOException
+    {
+        try
+        {
+            return dir.toPath().toRealPath();
+        }
+        catch (NoSuchFileException e)
+        {
+            return dir.toPath().toAbsolutePath().normalize();
+        }
+    }
+
+    /**
+     * Validates {@code uploadFieldId} and {@code clientFileName} against the base folder to
+     * prevent path traversal, and returns the fully resolved, canonical target file.
+     *
+     * @throws SecurityException if either component would escape the base folder
+     */
+    private java.io.File resolveTargetFile(String uploadFieldId, String clientFileName)
+            throws IOException
+    {
+        Path baseFolderPath = getFolder().toPath().toRealPath();
+        java.io.File uploadFieldFolder = new File(getFolder(), uploadFieldId).getCanonicalFile();
+        if (!uploadFieldFolder.toPath().startsWith(baseFolderPath))
+        {
+            throw new SecurityException("Path traversal detected in uploadFieldId");
+        }
+        java.io.File target = new File(uploadFieldFolder, clientFileName).getCanonicalFile();
+        Path uploadFieldFolderPath = getPathForComparison(uploadFieldFolder);
+        if (!target.toPath().startsWith(uploadFieldFolderPath))
+        {
+            throw new SecurityException("Path traversal detected in client filename");
+        }
+        return target;
+    }
+
     @Override
     public void save(FileUpload fileItem, String uploadFieldId)
     {
-        File uploadFieldFolder = new File(getFolder(), uploadFieldId);
-        uploadFieldFolder.mkdirs();
         try
         {
-            IOUtils.copy(fileItem.getInputStream(),  new FileOutputStream(new File(uploadFieldFolder, fileItem.getClientFileName())));
+            java.io.File target = resolveTargetFile(uploadFieldId, fileItem.getClientFileName());
+            Files.createDirectories(target.toPath().getParent());
+            try (InputStream in = fileItem.getInputStream();
+                 FileOutputStream out = new FileOutputStream(target))
+            {
+                IOUtils.copy(in, out);
+            }
         }
         catch (IOException e)
         {
@@ -78,6 +129,13 @@ public void save(FileUpload fileItem, String uploadFieldId)
     @Override
     public File getFile(String uploadFieldId, String clientFileName)
     {
-        return new File(new File(getFolder(), uploadFieldId), clientFileName);
+        try
+        {
+            return new File(resolveTargetFile(uploadFieldId, clientFileName));
+        }
+        catch (IOException e)
+        {
+            throw new WicketRuntimeException(e);
+        }
     }
 }