diff --git a/commons-fileupload2-core/src/main/java/org/apache/commons/fileupload2/core/DeferrableOutputStream.java b/commons-fileupload2-core/src/main/java/org/apache/commons/fileupload2/core/DeferrableOutputStream.java index a87625fc2..c75ad5d0e 100644 --- a/commons-fileupload2-core/src/main/java/org/apache/commons/fileupload2/core/DeferrableOutputStream.java +++ b/commons-fileupload2-core/src/main/java/org/apache/commons/fileupload2/core/DeferrableOutputStream.java @@ -21,8 +21,12 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.channels.Channels; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.EnumSet; import java.util.function.Supplier; /** @@ -388,7 +392,16 @@ protected OutputStream persist() throws IOException { if (dir != null) { Files.createDirectories(dir); } - final OutputStream os = Files.newOutputStream(p); + // Restrict the temporary file to its owner where the file system supports it. The default repository is the shared system temporary directory, so + // creating the file with default permissions would expose the uploaded data to other local users. + final EnumSet options = EnumSet.of(StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE); + final OutputStream os; + if (p.getFileSystem().supportedFileAttributeViews().contains("posix")) { + os = Channels.newOutputStream(Files.newByteChannel(p, options, + PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------")))); + } else { + os = Channels.newOutputStream(Files.newByteChannel(p, options)); + } if (baos != null) { baos.writeTo(os); } diff --git a/commons-fileupload2-core/src/test/java/org/apache/commons/fileupload2/core/DeferrableOutputStreamTest.java b/commons-fileupload2-core/src/test/java/org/apache/commons/fileupload2/core/DeferrableOutputStreamTest.java index 0d457b208..e0c5f9fc3 100644 --- a/commons-fileupload2-core/src/test/java/org/apache/commons/fileupload2/core/DeferrableOutputStreamTest.java +++ b/commons-fileupload2-core/src/test/java/org/apache/commons/fileupload2/core/DeferrableOutputStreamTest.java @@ -20,8 +20,10 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -29,9 +31,11 @@ import java.io.OutputStream; import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; +import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.attribute.PosixFilePermissions; import java.util.function.Consumer; import java.util.function.Supplier; @@ -240,4 +244,35 @@ void testThresholdZero() { } }); } + + /** + * Tests that the temporary file is created with owner-only permissions on POSIX file systems, + * so that uploaded data in the shared system temporary directory is not readable by other users. + */ + @Test + void testPersistedFileIsOwnerOnlyOnPosix() throws IOException { + assumeTrue(FileSystems.getDefault().supportedFileAttributeViews().contains("posix")); + // Mirror production, where the supplier only resolves a not-yet-existing path. + final Path target = tempTestDir.resolve("posixPerms.bin"); + try (final DeferrableOutputStream dos = new DeferrableOutputStream(-1, () -> target, null)) { + dos.write("secret".getBytes(StandardCharsets.UTF_8)); + } + assertTrue(Files.isRegularFile(target)); + assertEquals("rw-------", PosixFilePermissions.toString(Files.getPosixFilePermissions(target))); + } + + /** + * Tests that persisting over an already existing path truncates and overwrites it, matching the prior + * {@link Files#newOutputStream} behavior. Guards against a regression if {@code TRUNCATE_EXISTING} is dropped. + */ + @Test + void testPersistedFileOverwritesExisting() throws IOException { + final Path target = tempTestDir.resolve("overwrite.bin"); + Files.write(target, "stale-and-longer".getBytes(StandardCharsets.UTF_8)); + try (final DeferrableOutputStream dos = new DeferrableOutputStream(-1, () -> target, null)) { + dos.write("new".getBytes(StandardCharsets.UTF_8)); + } + assertTrue(Files.isRegularFile(target)); + assertArrayEquals("new".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(target)); + } }