This is an automated email from the ASF dual-hosted git repository.
fanningpj 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 45a3e16e7e add IOUtils.newFile(parent, path) (#855)
45a3e16e7e is described below
commit 45a3e16e7e9518ea8c7ecc85d91c8d37fd61bb39
Author: PJ Fanning <[email protected]>
AuthorDate: Wed Jul 23 12:43:18 2025 +0100
add IOUtils.newFile(parent, path) (#855)
* add IOUtils.newFile(parent, path)
* Update IOUtils.java
* Update IOUtils.java
---
.../java/org/apache/poi/poifs/dev/POIFSDump.java | 12 +++--
.../apache/poi/poifs/macros/VBAMacroExtractor.java | 3 +-
poi/src/main/java/org/apache/poi/util/IOUtils.java | 23 ++++++++-
.../test/java/org/apache/poi/util/TestIOUtils.java | 54 ++++++++++++++++++++++
4 files changed, 85 insertions(+), 7 deletions(-)
diff --git a/poi/src/main/java/org/apache/poi/poifs/dev/POIFSDump.java
b/poi/src/main/java/org/apache/poi/poifs/dev/POIFSDump.java
index d29745ea82..fdf6bb85f4 100644
--- a/poi/src/main/java/org/apache/poi/poifs/dev/POIFSDump.java
+++ b/poi/src/main/java/org/apache/poi/poifs/dev/POIFSDump.java
@@ -71,7 +71,7 @@ public final class POIFSDump {
DirectoryEntry root = fs.getRoot();
String filenameWithoutPath = new File(filename).getName();
File dumpDir = new File(filenameWithoutPath + "_dump");
- File file = new File(dumpDir, root.getName());
+ File file = IOUtils.newFile(dumpDir, root.getName());
if (!file.exists() && !file.mkdirs()) {
throw new IOException("Could not create directory " +
file);
}
@@ -104,13 +104,14 @@ public final class POIFSDump {
try (DocumentInputStream is = new DocumentInputStream(node)) {
bytes = IOUtils.toByteArray(is);
}
- try (OutputStream out = Files.newOutputStream(new File(parent,
node.getName().trim()).toPath())) {
+ try (OutputStream out = Files.newOutputStream(
+ IOUtils.newFile(parent,
node.getName().trim()).toPath())) {
out.write(bytes);
}
} else if (entry instanceof DirectoryEntry){
DirectoryEntry dir = (DirectoryEntry)entry;
- File file = new File(parent, entry.getName());
- if(!file.exists() && !file.mkdirs()) {
+ File file = IOUtils.newFile(parent, entry.getName());
+ if (!file.exists() && !file.mkdirs()) {
throw new IOException("Could not create directory " +
file);
}
dump(dir, file);
@@ -119,8 +120,9 @@ public final class POIFSDump {
}
}
}
+
public static void dump(POIFSFileSystem fs, int startBlock, String name,
File parent) throws IOException {
- File file = new File(parent, name);
+ File file = IOUtils.newFile(parent, name);
try (OutputStream out = Files.newOutputStream(file.toPath())) {
POIFSStream stream = new POIFSStream(fs, startBlock);
diff --git
a/poi/src/main/java/org/apache/poi/poifs/macros/VBAMacroExtractor.java
b/poi/src/main/java/org/apache/poi/poifs/macros/VBAMacroExtractor.java
index c0db47e5fb..e5a80e9034 100644
--- a/poi/src/main/java/org/apache/poi/poifs/macros/VBAMacroExtractor.java
+++ b/poi/src/main/java/org/apache/poi/poifs/macros/VBAMacroExtractor.java
@@ -26,6 +26,7 @@ import java.nio.file.Files;
import java.util.Map;
import java.util.Map.Entry;
+import org.apache.poi.util.IOUtils;
import org.apache.poi.util.StringUtil;
/**
@@ -95,7 +96,7 @@ public class VBAMacroExtractor {
System.out.println();
System.out.println(moduleCode);
} else {
- File out = new File(outputDir, moduleName + extension);
+ File out = IOUtils.newFile(outputDir, moduleName + extension);
try (OutputStream fout = Files.newOutputStream(out.toPath());
OutputStreamWriter fwriter = new OutputStreamWriter(fout,
StringUtil.UTF8)) {
fwriter.write(moduleCode);
diff --git a/poi/src/main/java/org/apache/poi/util/IOUtils.java
b/poi/src/main/java/org/apache/poi/util/IOUtils.java
index ff86043a54..b913a9bf03 100644
--- a/poi/src/main/java/org/apache/poi/util/IOUtils.java
+++ b/poi/src/main/java/org/apache/poi/util/IOUtils.java
@@ -601,7 +601,6 @@ public final class IOUtils {
return Arrays.copyOfRange(src, offset, offset+realLength);
}
-
/**
* Simple utility function to check that you haven't hit EOF
* when reading a byte.
@@ -618,6 +617,28 @@ public final class IOUtils {
return b;
}
+ /**
+ * Creates a new file in the given parent directory with the given name.
+ * There is a check to prevent path traversal attacks. Only path traversal
+ * that would lead to a file outside the parent directory is regarded as
an issue.
+ *
+ * @param parent The parent directory where the file should be created.
+ * @param name The name of the file to create.
+ * @return The created file.
+ * @throws IOException If path traversal is detected.
+ * @since POI 5.4.2
+ */
+ public static File newFile(final File parent, final String name) throws
IOException {
+ final File file = new File(parent, name);
+ if (!file.toPath().toAbsolutePath().normalize().startsWith(
+ parent.toPath().toAbsolutePath().normalize()
+ )) {
+ throw new IOException(String.format(
+ Locale.ROOT, "Failing due to path traversal in `%s`",
name));
+ }
+ return file;
+ }
+
private static void throwRFE(long length, int maxLength) {
throw new RecordFormatException(String.format(Locale.ROOT, "Tried to
allocate an array of length %,d" +
", but the maximum length for this record type is
%,d.%n" +
diff --git a/poi/src/test/java/org/apache/poi/util/TestIOUtils.java
b/poi/src/test/java/org/apache/poi/util/TestIOUtils.java
index 7f026adc74..094e138eef 100644
--- a/poi/src/test/java/org/apache/poi/util/TestIOUtils.java
+++ b/poi/src/test/java/org/apache/poi/util/TestIOUtils.java
@@ -41,6 +41,7 @@ import java.nio.charset.StandardCharsets;
import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream;
import org.apache.poi.EmptyFileException;
import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.parallel.Isolated;
@@ -575,6 +576,59 @@ final class TestIOUtils {
assertEquals(4, ret.length);
}
+ @Test
+ void testNewFile() throws IOException {
+ final File parent = TempFile.createTempDirectory("create-file-test");
+ try {
+ final String path0 = "path/to/file.txt";
+ final File outFile = IOUtils.newFile(parent, path0);
+ assertTrue(outFile.getAbsolutePath().endsWith(path0),
+ "unexpected path: " + outFile.getAbsolutePath());
+ } finally {
+ assertTrue(parent.delete());
+ }
+ }
+
+ @Test
+ void testAllowedPathTraversal() throws IOException {
+ final File parent =
TempFile.createTempDirectory("path-traversal-test");
+ try {
+ // this path is ok because it doesn't walk out of the parent
directory
+ final String path0 = "a/b/c/../d/e/../../f/g/./h";
+ File outFile = IOUtils.newFile(parent, path0);
+ assertTrue(outFile.getAbsolutePath().endsWith(path0),
+ "unexpected path: " + outFile.getAbsolutePath());
+ } finally {
+ assertTrue(parent.delete());
+ }
+ }
+
+ @Test
+ void testAllowedPathTraversal2() throws IOException {
+ final File parent =
TempFile.createTempDirectory("path-traversal-test");
+ try {
+ // this path is ok because it doesn't walk out of the parent
directory
+ // the initial slash is ignored and the generated path is relative
to the parent directory
+ final String path0 = "/a/b/c.txt";
+ File outFile = IOUtils.newFile(parent, path0);
+ assertTrue(outFile.getAbsolutePath().endsWith(path0),
+ "unexpected path: " + outFile.getAbsolutePath());
+ } finally {
+ assertTrue(parent.delete());
+ }
+ }
+
+ @Test
+ void testDisallowedPathTraversal() throws IOException {
+ final File parent =
TempFile.createTempDirectory("path-traversal-test");
+ try {
+ final String path0 = "../a/b/c.txt";
+ Assertions.assertThrows(IOException.class, () ->
IOUtils.newFile(parent, path0));
+ } finally {
+ assertTrue(parent.delete());
+ }
+ }
+
/**
* This returns 0 for the first call to skip and then reads
* as requested. This tests that the fallback to read() works.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
