You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by fa...@apache.org on 2026/08/01 18:18:14 UTC
(poi) branch trunk updated: VBA reader order (#1197)
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 6d55b755db VBA reader order (#1197)
6d55b755db is described below
commit 6d55b755db1429b3342e5a0dabe190cef7278468
Author: PJ Fanning <pj...@users.noreply.github.com>
AuthorDate: Sat Aug 1 19:18:09 2026 +0100
VBA reader order (#1197)
* Update VBAMacroReader.java
* Update TestVBAMacroReader.java
* Update VBAMacroReader.java
* Update VBAMacroReader.java
* Update VBAMacroReader.java
---
.../apache/poi/poifs/macros/VBAMacroReader.java | 73 ++++++------
.../poi/poifs/macros/TestVBAMacroReader.java | 124 +++++++++++++++++++++
2 files changed, 158 insertions(+), 39 deletions(-)
diff --git a/poi/src/main/java/org/apache/poi/poifs/macros/VBAMacroReader.java b/poi/src/main/java/org/apache/poi/poifs/macros/VBAMacroReader.java
index 7edadea0f5..dcc3a58f26 100644
--- a/poi/src/main/java/org/apache/poi/poifs/macros/VBAMacroReader.java
+++ b/poi/src/main/java/org/apache/poi/poifs/macros/VBAMacroReader.java
@@ -29,7 +29,11 @@ import java.io.InputStreamReader;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
+import java.util.Arrays;
import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Set;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.zip.ZipEntry;
@@ -120,26 +124,38 @@ public class VBAMacroReader implements Closeable {
this.fs = fs;
}
+ private static final Set<String> CANONICAL_VBA_DIRS =
+ Set.of("xl/", "word/", "ppt/", "visio/");
+
private void openOOXML(InputStream zipFile) throws IOException {
- try(ZipInputStream zis = new ZipInputStream(zipFile)) {
+ byte[] fallback = null;
+ try (ZipInputStream zis = new ZipInputStream(zipFile)) {
ZipEntry zipEntry;
while ((zipEntry = zis.getNextEntry()) != null) {
if (endsWithIgnoreCase(zipEntry.getName(), VBA_PROJECT_OOXML)) {
- try {
- // Make a POIFSFileSystem from the contents, and close the stream
- this.fs = new POIFSFileSystem(zis);
+ byte[] bytes = IOUtils.toByteArray(zis);
+ String lowerName = zipEntry.getName().toLowerCase(Locale.ROOT);
+ boolean canonical = CANONICAL_VBA_DIRS.stream()
+ .anyMatch(lowerName::startsWith);
+ if (canonical) {
+ // Found vbaProject.bin at a standard OPC path.
+ // Prefer this over any earlier non-canonical match.
+ this.fs = new POIFSFileSystem(
+ UnsynchronizedByteArrayInputStream.builder().setByteArray(bytes).get());
return;
- } catch (IOException e) {
- // Tidy up
- zis.close();
-
- // Pass on
- throw e;
+ }
+ // Non-canonical path — keep as fallback in case no canonical match appears
+ if (fallback == null) {
+ fallback = bytes;
}
}
}
}
- throw new IllegalArgumentException("No VBA project found");
+ if (fallback == null) {
+ throw new IllegalArgumentException("No VBA project found");
+ }
+ this.fs = new POIFSFileSystem(
+ UnsynchronizedByteArrayInputStream.builder().setByteArray(fallback).get());
}
@Override
@@ -474,40 +490,19 @@ public class VBAMacroReader implements Closeable {
}
}
-
private enum DIR_STATE {
INFORMATION_RECORD,
REFERENCES_RECORD,
MODULES_RECORD
}
- private static class ASCIIUnicodeStringPair {
- private final String ascii;
- private final String unicode;
- private final int pushbackRecordId;
-
+ private record ASCIIUnicodeStringPair(String ascii, String unicode, int pushbackRecordId) {
ASCIIUnicodeStringPair(String ascii, int pushbackRecordId) {
- this.ascii = ascii;
- this.unicode = "";
- this.pushbackRecordId = pushbackRecordId;
+ this(ascii, "", pushbackRecordId);
}
ASCIIUnicodeStringPair(String ascii, String unicode) {
- this.ascii = ascii;
- this.unicode = unicode;
- pushbackRecordId = -1;
- }
-
- private String getAscii() {
- return ascii;
- }
-
- private String getUnicode() {
- return unicode;
- }
-
- private int getPushbackRecordId() {
- return pushbackRecordId;
+ this(ascii, unicode, -1);
}
}
@@ -540,7 +535,7 @@ public class VBAMacroReader implements Closeable {
break;
case MODULE_STREAM_NAME:
ASCIIUnicodeStringPair pair = readStringPair(in, modules.charset, STREAMNAME_RESERVED);
- streamName = pair.getAscii();
+ streamName = pair.ascii();
break;
case PROJECT_DOC_STRING:
readStringPair(in, modules.charset, DOC_STRING_RESERVED);
@@ -557,18 +552,18 @@ public class VBAMacroReader implements Closeable {
}
ASCIIUnicodeStringPair stringPair = readStringPair(in,
modules.charset, REFERENCE_NAME_RESERVED, false);
- if (stringPair.getPushbackRecordId() == -1) {
+ if (stringPair.pushbackRecordId() == -1) {
break;
}
//Special handling for when there's only an ascii string and a REFERENCED_REGISTERED
//record that follows.
//See https://github.com/decalage2/oletools/blob/master/oletools/olevba.py#L1516
//and https://github.com/decalage2/oletools/pull/135 from (@c1fe)
- if (stringPair.getPushbackRecordId() != RecordType.REFERENCE_REGISTERED.id) {
+ if (stringPair.pushbackRecordId() != RecordType.REFERENCE_REGISTERED.id) {
throw new IllegalArgumentException("Unexpected reserved character. "+
"Expected "+Integer.toHexString(REFERENCE_NAME_RESERVED)
+ " or "+Integer.toHexString(RecordType.REFERENCE_REGISTERED.id)+
- " not: "+Integer.toHexString(stringPair.getPushbackRecordId()));
+ " not: "+Integer.toHexString(stringPair.pushbackRecordId()));
}
//fall through!
case REFERENCE_REGISTERED:
diff --git a/poi/src/test/java/org/apache/poi/poifs/macros/TestVBAMacroReader.java b/poi/src/test/java/org/apache/poi/poifs/macros/TestVBAMacroReader.java
index e867d3cf80..1f2cdd7574 100644
--- a/poi/src/test/java/org/apache/poi/poifs/macros/TestVBAMacroReader.java
+++ b/poi/src/test/java/org/apache/poi/poifs/macros/TestVBAMacroReader.java
@@ -33,13 +33,20 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.poi.util.RecordFormatException;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
+import java.util.TreeMap;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
+import java.util.zip.ZipOutputStream;
class TestVBAMacroReader {
private static final Map<POIDataSamples, String> expectedMacroContents;
@@ -344,4 +351,121 @@ class TestVBAMacroReader {
VBAMacroReader.setMaxStringLength(prevLimit);
}
}
+
+ /**
+ * Verify that VBAMacroReader prefers the vbaProject.bin at a canonical OPC path
+ * (xl/vbaProject.bin) over one at a non-canonical path (docProps/vbaProject.bin),
+ * regardless of ZIP stream order.
+ *
+ * This prevents evasion where a crafted file places a benign decoy VBA project at a
+ * non-standard path that appears earlier in the ZIP stream than the real project.
+ */
+ @Test
+ void prefersCanonicalVbaProjectPath() throws IOException {
+ File realSrc = POIDataSamples.getSpreadSheetInstance().getFile("SimpleMacro.xlsm");
+ File decoySrc = POIDataSamples.getSpreadSheetInstance().getFile("testNames.xlsm");
+
+ byte[] realVba = extractZipEntry(realSrc, "xl/vbaProject.bin");
+ byte[] decoyVba = extractZipEntry(decoySrc, "xl/vbaProject.bin");
+
+ // Ground-truth: what each project's macros look like
+ Map<String, String> realMacros;
+ try (VBAMacroReader r = new VBAMacroReader(realSrc)) {
+ realMacros = new TreeMap<>(r.readMacros());
+ }
+ Map<String, String> decoyMacros;
+ try (VBAMacroReader r = new VBAMacroReader(decoySrc)) {
+ decoyMacros = new TreeMap<>(r.readMacros());
+ }
+ assertFalse(realMacros.equals(decoyMacros), "test fixtures must have different macros");
+
+ // Craft an .xlsm with decoy at docProps/vbaProject.bin (non-canonical, first in stream)
+ // and real at xl/vbaProject.bin (canonical, second in stream).
+ byte[] crafted = craftDualVbaXlsm(realVba, decoyVba, /*decoyFirst=*/true);
+
+ // VBAMacroReader should return the REAL macros (from the canonical xl/ path),
+ // not the decoy (from the non-canonical docProps/ path).
+ Map<String, String> seen;
+ try (VBAMacroReader r = new VBAMacroReader(new ByteArrayInputStream(crafted))) {
+ seen = new TreeMap<>(r.readMacros());
+ }
+ assertEquals(realMacros, seen,
+ "VBAMacroReader must prefer canonical xl/vbaProject.bin over non-canonical docProps/vbaProject.bin");
+
+ // Reverse order: real first, decoy second — should still get real macros.
+ byte[] craftedReverse = craftDualVbaXlsm(realVba, decoyVba, /*decoyFirst=*/false);
+ Map<String, String> seenReverse;
+ try (VBAMacroReader r = new VBAMacroReader(new ByteArrayInputStream(craftedReverse))) {
+ seenReverse = new TreeMap<>(r.readMacros());
+ }
+ assertEquals(realMacros, seenReverse,
+ "VBAMacroReader must return canonical xl/vbaProject.bin regardless of stream order");
+ }
+
+ private static byte[] extractZipEntry(File zip, String entryName) throws IOException {
+ try (ZipFile zf = new ZipFile(zip)) {
+ ZipEntry e = zf.getEntry(entryName);
+ assertNotNull(e, "entry '" + entryName + "' not in " + zip);
+ try (InputStream in = zf.getInputStream(e)) {
+ return in.readAllBytes();
+ }
+ }
+ }
+
+ /**
+ * Assemble a minimal .xlsm with two vbaProject.bin entries:
+ * real at xl/vbaProject.bin (canonical) and decoy at docProps/vbaProject.bin (non-canonical).
+ * Stream order is controlled by {@code decoyFirst}.
+ */
+ private static byte[] craftDualVbaXlsm(byte[] realVba, byte[] decoyVba, boolean decoyFirst) throws IOException {
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ try (ZipOutputStream zos = new ZipOutputStream(bos)) {
+ putZipEntry(zos, "[Content_Types].xml",
+ ("<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>"
+ + "<Types xmlns=\"http://schemas.openxmlformats.org/package/2006/content-types\">"
+ + "<Default Extension=\"rels\" ContentType=\"application/vnd.openxmlformats-package.relationships+xml\"/>"
+ + "<Default Extension=\"xml\" ContentType=\"application/xml\"/>"
+ + "<Default Extension=\"bin\" ContentType=\"application/vnd.ms-office.vbaProject\"/>"
+ + "<Override PartName=\"/xl/workbook.xml\" ContentType=\"application/vnd.ms-excel.sheet.macroEnabled.main+xml\"/>"
+ + "</Types>").getBytes(StandardCharsets.UTF_8));
+
+ putZipEntry(zos, "_rels/.rels",
+ ("<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>"
+ + "<Relationships xmlns=\"http://schemas.openxmlformats.org/package/2006/relationships\">"
+ + "<Relationship Id=\"rId1\" "
+ + "Type=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships/officeDocument\" "
+ + "Target=\"xl/workbook.xml\"/>"
+ + "</Relationships>").getBytes(StandardCharsets.UTF_8));
+
+ putZipEntry(zos, "xl/workbook.xml",
+ ("<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>"
+ + "<workbook xmlns=\"http://schemas.openxmlformats.org/spreadsheetml/2006/main\" "
+ + "xmlns:r=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships\">"
+ + "<sheets><sheet name=\"Sheet1\" sheetId=\"1\" r:id=\"rId1\"/></sheets>"
+ + "</workbook>").getBytes(StandardCharsets.UTF_8));
+
+ putZipEntry(zos, "xl/_rels/workbook.xml.rels",
+ ("<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>"
+ + "<Relationships xmlns=\"http://schemas.openxmlformats.org/package/2006/relationships\">"
+ + "<Relationship Id=\"rIdVba\" "
+ + "Type=\"http://schemas.microsoft.com/office/2006/relationships/vbaProject\" "
+ + "Target=\"vbaProject.bin\"/>"
+ + "</Relationships>").getBytes(StandardCharsets.UTF_8));
+
+ if (decoyFirst) {
+ putZipEntry(zos, "docProps/vbaProject.bin", decoyVba);
+ putZipEntry(zos, "xl/vbaProject.bin", realVba);
+ } else {
+ putZipEntry(zos, "xl/vbaProject.bin", realVba);
+ putZipEntry(zos, "docProps/vbaProject.bin", decoyVba);
+ }
+ }
+ return bos.toByteArray();
+ }
+
+ private static void putZipEntry(ZipOutputStream zos, String name, byte[] data) throws IOException {
+ zos.putNextEntry(new ZipEntry(name));
+ zos.write(data);
+ zos.closeEntry();
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org