add IOUtils.newFile(parent, path) (#855)

* add IOUtils.newFile(parent, path)

* Update IOUtils.java

* Update IOUtils.java
This commit is contained in:
PJ Fanning 2025-07-23 12:43:18 +01:00 committed by GitHub
parent 1a86f27e7b
commit 45a3e16e7e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 85 additions and 7 deletions

View File

@ -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);

View File

@ -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);

View File

@ -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" +

View File

@ -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.