This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/poi.git
The following commit(s) were added to refs/heads/trunk by this push:
new 7e761ad622 Harden temporary file permissions (#1063)
7e761ad622 is described below
commit 7e761ad6228e8f8bf05c9e9e485d93e78e2829e5
Author: metsw24-max <[email protected]>
AuthorDate: Tue May 12 15:33:53 2026 +0530
Harden temporary file permissions (#1063)
---
.../poi/util/DefaultTempFileCreationStrategy.java | 61 +++++++++++++++++++++-
.../util/DefaultTempFileCreationStrategyTest.java | 33 ++++++++++++
2 files changed, 92 insertions(+), 2 deletions(-)
diff --git
a/poi/src/main/java/org/apache/poi/util/DefaultTempFileCreationStrategy.java
b/poi/src/main/java/org/apache/poi/util/DefaultTempFileCreationStrategy.java
index 4f047ecf2c..5d90b94d76 100644
--- a/poi/src/main/java/org/apache/poi/util/DefaultTempFileCreationStrategy.java
+++ b/poi/src/main/java/org/apache/poi/util/DefaultTempFileCreationStrategy.java
@@ -24,6 +24,9 @@ import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.FileAttribute;
+import java.nio.file.attribute.PosixFilePermission;
+import java.nio.file.attribute.PosixFilePermissions;
+import java.util.Set;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
@@ -92,7 +95,27 @@ public class DefaultTempFileCreationStrategy implements
TempFileCreationStrategy
createPOIFilesDirectoryIfNecessary();
// Generate a unique new filename
- File newFile = Files.createTempFile(dir.toPath(), prefix,
suffix).toFile();
+ File newFile;
+ try {
+ // Try POSIX permissions first (owner read/write only)
+ Path p = Files.createTempFile(dir.toPath(), prefix, suffix,
+
PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------")));
+ newFile = p.toFile();
+ } catch (UnsupportedOperationException | IOException e) {
+ // POSIX not supported (e.g., Windows) or failed: fall back to
creating normally
+ newFile = Files.createTempFile(dir.toPath(), prefix,
suffix).toFile();
+ try {
+ // Clear all perms for everyone, then set owner-only perms
where supported
+ newFile.setReadable(false, false);
+ newFile.setWritable(false, false);
+ newFile.setExecutable(false, false);
+ newFile.setReadable(true, true);
+ newFile.setWritable(true, true);
+ newFile.setExecutable(false, true);
+ } catch (Exception ignore) {
+ // best-effort only
+ }
+ }
// Set the delete on exit flag if sys prop is set
if (System.getProperty(DELETE_FILES_ON_EXIT) != null) {
@@ -110,7 +133,24 @@ public class DefaultTempFileCreationStrategy implements
TempFileCreationStrategy
createPOIFilesDirectoryIfNecessary();
// Generate a unique new filename
- File newDirectory = Files.createTempDirectory(dir.toPath(),
prefix).toFile();
+ File newDirectory;
+ try {
+ Path p = Files.createTempDirectory(dir.toPath(), prefix,
+
PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwx------")));
+ newDirectory = p.toFile();
+ } catch (UnsupportedOperationException | IOException e) {
+ newDirectory = Files.createTempDirectory(dir.toPath(),
prefix).toFile();
+ try {
+ newDirectory.setReadable(false, false);
+ newDirectory.setWritable(false, false);
+ newDirectory.setExecutable(false, false);
+ newDirectory.setReadable(true, true);
+ newDirectory.setWritable(true, true);
+ newDirectory.setExecutable(true, true);
+ } catch (Exception ignore) {
+ // best-effort only
+ }
+ }
//this method appears to be only used in tests, so it is probably ok
to use deleteOnExit
newDirectory.deleteOnExit();
@@ -155,6 +195,23 @@ public class DefaultTempFileCreationStrategy implements
TempFileCreationStrategy
dir = fileDir;
} else {
dir = Files.createDirectories(dirPath).toFile();
+ try {
+ // attempt to restrict directory perms to owner
only
+ try {
+ Set<PosixFilePermission> perms =
PosixFilePermissions.fromString("rwx------");
+ Files.setPosixFilePermissions(dir.toPath(),
perms);
+ } catch (UnsupportedOperationException |
IOException e) {
+ // fallback: use File API to set owner-only
flags where supported
+ dir.setReadable(false, false);
+ dir.setWritable(false, false);
+ dir.setExecutable(false, false);
+ dir.setReadable(true, true);
+ dir.setWritable(true, true);
+ dir.setExecutable(true, true);
+ }
+ } catch (Exception ignored) {
+ // best-effort only
+ }
}
}
} finally {
diff --git
a/poi/src/test/java/org/apache/poi/util/DefaultTempFileCreationStrategyTest.java
b/poi/src/test/java/org/apache/poi/util/DefaultTempFileCreationStrategyTest.java
index d60e4da8ac..c1d9d5c153 100644
---
a/poi/src/test/java/org/apache/poi/util/DefaultTempFileCreationStrategyTest.java
+++
b/poi/src/test/java/org/apache/poi/util/DefaultTempFileCreationStrategyTest.java
@@ -28,11 +28,16 @@ import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
+import java.nio.file.attribute.PosixFileAttributeView;
+import java.nio.file.attribute.PosixFilePermission;
+import java.util.Set;
import org.apache.commons.io.FileUtils;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.assertFalse;
class DefaultTempFileCreationStrategyTest {
@@ -214,4 +219,32 @@ class DefaultTempFileCreationStrategyTest {
assertTrue(file.delete());
}
}
+
+ @Test
+ void testPosixPermissions() throws IOException {
+ DefaultTempFileCreationStrategy strategy = new
DefaultTempFileCreationStrategy();
+ File dir = strategy.createTempDirectory("posixtest");
+ try {
+ Path dirPath = dir.toPath();
+ // Skip test if POSIX file attribute view not supported on this
filesystem
+ Assumptions.assumeTrue(Files.getFileAttributeView(dirPath,
PosixFileAttributeView.class) != null,
+ "POSIX file attributes not supported on this filesystem");
+
+ File f = strategy.createTempFile("posixtestfile", ".tmp");
+ try {
+ Set<PosixFilePermission> perms =
Files.getPosixFilePermissions(f.toPath());
+ // Owner should have read/write, no group/other perms
+ assertTrue(perms.contains(PosixFilePermission.OWNER_READ));
+ assertTrue(perms.contains(PosixFilePermission.OWNER_WRITE));
+ assertFalse(perms.contains(PosixFilePermission.GROUP_READ));
+ assertFalse(perms.contains(PosixFilePermission.GROUP_WRITE));
+ assertFalse(perms.contains(PosixFilePermission.OTHERS_READ));
+ assertFalse(perms.contains(PosixFilePermission.OTHERS_WRITE));
+ } finally {
+ assertTrue(f.delete());
+ }
+ } finally {
+ FileUtils.deleteDirectory(dir);
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
