aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--build.gradle6
-rw-r--r--build.xml14
-rw-r--r--poi-ooxml/src/main/java/org/apache/poi/xssf/model/ThemesTable.java54
-rw-r--r--poi/src/main/java/org/apache/poi/poifs/dev/POIFSDump.java12
-rw-r--r--poi/src/main/java/org/apache/poi/poifs/macros/VBAMacroExtractor.java3
-rw-r--r--poi/src/main/java/org/apache/poi/poifs/property/PropertyTable.java13
-rw-r--r--poi/src/main/java/org/apache/poi/util/IOUtils.java23
-rw-r--r--poi/src/test/java/org/apache/poi/util/TestIOUtils.java54
-rw-r--r--src/documentation/content/xdocs/changes.xml3
9 files changed, 135 insertions, 47 deletions
diff --git a/build.gradle b/build.gradle
index 6856adf9c8..a1a615919b 100644
--- a/build.gradle
+++ b/build.gradle
@@ -67,7 +67,7 @@ configurations {
}
dependencies {
- antLibs("org.junit.jupiter:junit-jupiter:5.13.3")
+ antLibs("org.junit.jupiter:junit-jupiter:5.13.4")
antLibs("org.apache.ant:ant-junitlauncher:1.10.15")
}
@@ -108,11 +108,11 @@ subprojects {
ext {
bouncyCastleVersion = '1.81'
- commonsCodecVersion = '1.18.0'
+ commonsCodecVersion = '1.19.0'
commonsCompressVersion = '1.27.1'
commonsIoVersion = '2.20.0'
commonsMathVersion = '3.6.1'
- junitVersion = '5.13.3'
+ junitVersion = '5.13.4'
log4jVersion = '2.24.3'
mockitoVersion = '4.11.0'
hamcrestVersion = '3.0'
diff --git a/build.xml b/build.xml
index bc41fe5a69..ad71e34971 100644
--- a/build.xml
+++ b/build.xml
@@ -263,21 +263,21 @@ under the License.
<!-- jars in the /lib directory, see the fetch-jars target-->
- <dependency prefix="main.commons-codec" artifact="commons-codec:commons-codec:1.18.0" usage="main"/>
+ <dependency prefix="main.commons-codec" artifact="commons-codec:commons-codec:1.19.0" usage="main"/>
<dependency prefix="main.commons-collections4" artifact="org.apache.commons:commons-collections4:4.5.0" usage="main"/>
<dependency prefix="main.commons-math3" artifact="org.apache.commons:commons-math3:3.6.1" usage="main"/>
<dependency prefix="main.commons-io" artifact="commons-io:commons-io:2.20.0" usage="main"/>
<dependency prefix="main.com.zaxxer" artifact="com.zaxxer:SparseBitSet:1.3" usage="main"/>
<dependency prefix="main.log4j-api" artifact="org.apache.logging.log4j:log4j-api:2.24.3" usage="main"/>
- <dependency prefix="main.junit-api" artifact="org.junit.jupiter:junit-jupiter-api:5.13.3" usage="main-tests"/>
- <dependency prefix="main.junit-jengine" artifact="org.junit.jupiter:junit-jupiter-engine:5.13.3" usage="main-tests"/>
- <dependency prefix="main.junit-params" artifact="org.junit.jupiter:junit-jupiter-params:5.13.3" usage="main-tests"/>
+ <dependency prefix="main.junit-api" artifact="org.junit.jupiter:junit-jupiter-api:5.13.4" usage="main-tests"/>
+ <dependency prefix="main.junit-jengine" artifact="org.junit.jupiter:junit-jupiter-engine:5.13.4" usage="main-tests"/>
+ <dependency prefix="main.junit-params" artifact="org.junit.jupiter:junit-jupiter-params:5.13.4" usage="main-tests"/>
<dependency prefix="main.junit-opentest4j" artifact="org.opentest4j:opentest4j:1.2.0" usage="main-tests"/>
<dependency prefix="main.junit-apiguardian" artifact="org.apiguardian:apiguardian-api:1.1.2" usage="main-tests"/>
- <dependency prefix="main.junit-pcommons" artifact="org.junit.platform:junit-platform-commons:1.13.3" usage="main-tests"/>
- <dependency prefix="main.junit-pengine" artifact="org.junit.platform:junit-platform-engine:1.13.3" usage="main-tests"/>
- <dependency prefix="main.junit-plauncher" artifact="org.junit.platform:junit-platform-launcher:1.13.3" usage="main-tests"/>
+ <dependency prefix="main.junit-pcommons" artifact="org.junit.platform:junit-platform-commons:1.13.4" usage="main-tests"/>
+ <dependency prefix="main.junit-pengine" artifact="org.junit.platform:junit-platform-engine:1.13.4" usage="main-tests"/>
+ <dependency prefix="main.junit-plauncher" artifact="org.junit.platform:junit-platform-launcher:1.13.4" usage="main-tests"/>
<dependency prefix="main.jmh" artifact="org.openjdk.jmh:jmh-core:1.35" usage="main-tests"/>
diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/model/ThemesTable.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/model/ThemesTable.java
index 7bd391a06b..4d6655d5fc 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/xssf/model/ThemesTable.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/model/ThemesTable.java
@@ -36,32 +36,34 @@ import org.openxmlformats.schemas.drawingml.x2006.main.ThemeDocument;
* colors and fonts.
*/
public class ThemesTable extends POIXMLDocumentPart implements Themes {
- public enum ThemeElement {
- LT1(0, "Lt1"),
- DK1(1,"Dk1"),
- LT2(2,"Lt2"),
- DK2(3,"Dk2"),
- ACCENT1(4,"Accent1"),
- ACCENT2(5,"Accent2"),
- ACCENT3(6,"Accent3"),
- ACCENT4(7,"Accent4"),
- ACCENT5(8,"Accent5"),
- ACCENT6(9,"Accent6"),
- HLINK(10,"Hlink"),
- FOLHLINK(11,"FolHlink"),
- UNKNOWN(-1,null);
-
- public static ThemeElement byId(int idx) {
- if (idx >= values().length || idx < 0) return UNKNOWN;
- return values()[idx];
- }
- ThemeElement(int idx, String name) {
- this.idx = idx;
- this.name = name;
- }
- public final int idx;
- public final String name;
- }
+ public enum ThemeElement {
+ LT1(0, "Lt1"),
+ DK1(1, "Dk1"),
+ LT2(2, "Lt2"),
+ DK2(3, "Dk2"),
+ ACCENT1(4, "Accent1"),
+ ACCENT2(5, "Accent2"),
+ ACCENT3(6, "Accent3"),
+ ACCENT4(7, "Accent4"),
+ ACCENT5(8, "Accent5"),
+ ACCENT6(9, "Accent6"),
+ HLINK(10, "Hlink"),
+ FOLHLINK(11, "FolHlink"),
+ UNKNOWN(-1, null);
+
+ public static ThemeElement byId(int idx) {
+ if (idx >= values().length || idx < 0) return UNKNOWN;
+ return values()[idx];
+ }
+
+ ThemeElement(int idx, String name) {
+ this.idx = idx;
+ this.name = name;
+ }
+
+ public final int idx;
+ public final String name;
+ }
private IndexedColorMap colorMap;
private ThemeDocument theme;
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/poifs/property/PropertyTable.java b/poi/src/main/java/org/apache/poi/poifs/property/PropertyTable.java
index 062627ae52..562413e0c6 100644
--- a/poi/src/main/java/org/apache/poi/poifs/property/PropertyTable.java
+++ b/poi/src/main/java/org/apache/poi/poifs/property/PropertyTable.java
@@ -111,7 +111,7 @@ public final class PropertyTable implements BATManaged {
Property property = _properties.get(0);
if (property != null) {
if (property instanceof DirectoryProperty) {
- populatePropertyTree((DirectoryProperty) property);
+ populatePropertyTree((DirectoryProperty) property, 0);
} else {
throw new IOException("Invalid format, cannot convert property " + property + " to DirectoryProperty");
}
@@ -227,7 +227,14 @@ public final class PropertyTable implements BATManaged {
_header_block.setPropertyCount(countBlocks());
}
- private void populatePropertyTree(DirectoryProperty root) throws IOException {
+ // Maximum depth of the property tree to prevent stackoverflow errors
+ private static final int MAX_PROPERTY_DEPTH = 1000;
+
+ private void populatePropertyTree(final DirectoryProperty root, final int depth) throws IOException {
+ if (depth > MAX_PROPERTY_DEPTH) {
+ throw new IOException("Property tree too deep, likely a corrupt file");
+ }
+
int index = root.getChildIndex();
if (!Property.isValidIndex(index)) {
@@ -246,7 +253,7 @@ public final class PropertyTable implements BATManaged {
root.addChild(property);
if (property.isDirectory()) {
- populatePropertyTree(( DirectoryProperty ) property);
+ populatePropertyTree((DirectoryProperty) property, depth + 1);
}
index = property.getPreviousChildIndex();
if (isValidIndex(index)) {
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.
diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml
index c137381f78..d7fdc8b81c 100644
--- a/src/documentation/content/xdocs/changes.xml
+++ b/src/documentation/content/xdocs/changes.xml
@@ -73,7 +73,8 @@
<release version="5.4.2" date="2025-??-??">
<summary>
<summary-item>Upgrade batik dependency to 1.19</summary-item>
- <summary-item>Upgrade bouncycastle dependency to 1.80</summary-item>
+ <summary-item>Upgrade bouncycastle dependency to 1.81</summary-item>
+ <summary-item>Upgrade commons-codec dependency to 1.19.0</summary-item>
<summary-item>Upgrade commons-collections4 dependency to 4.5.0</summary-item>
<summary-item>Upgrade commons-io dependency to 2.20.0</summary-item>
<summary-item>Upgrade pdfbox dependency to 3.0.5</summary-item>
generated by cgit v1.2.3 (git 2.39.1) at 2025-07-23 18:38:17 +0000