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/09/18 19:29:10 UTC
(poi) branch trunk updated: [bug-66245] HWPF: no ArrayIndexOutOfBoundsException from truncated SPRMs (#1304)
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 b66d71f850 [bug-66245] HWPF: no ArrayIndexOutOfBoundsException from truncated SPRMs (#1304)
b66d71f850 is described below
commit b66d71f85030b060df543702d8c0d64dfac922c3
Author: PJ Fanning <pj...@users.noreply.github.com>
AuthorDate: Fri Sep 18 20:29:03 2026 +0100
[bug-66245] HWPF: no ArrayIndexOutOfBoundsException from truncated SPRMs (#1304)
A variable-length SPRM (size code 6) at the very end of a grpprl has no
room for its operand-length byte(s); SprmOperation.initSize read past
the array. Treat the remaining bytes as the SPRM's size so the iterator
stops there, and log a warning.
Also harden the neighbouring code paths for the same class of input:
- SprmOperation.getOperand size code 6: no read past the grpprl or past
the 4 bytes that fit an int (off-by-one bounds check, unchecked
operand length)
- SectionSprmUncompressor: log and skip an unapplyable SPRM like the
PAP/CHP/TAP uncompressors already do
- TableSprmUncompressor: sprmTDefTable with no operand bytes falls back
to the one-column default
Co-authored-by: Claude Opus 5 (1M context) <no...@anthropic.com>
---
.../poi/hwpf/sprm/SectionSprmUncompressor.java | 9 +-
.../org/apache/poi/hwpf/sprm/SprmOperation.java | 36 +++++-
.../poi/hwpf/sprm/TableSprmUncompressor.java | 7 +-
.../apache/poi/hwpf/sprm/TestSprmOperation.java | 126 +++++++++++++++++++++
4 files changed, 171 insertions(+), 7 deletions(-)
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/sprm/SectionSprmUncompressor.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/sprm/SectionSprmUncompressor.java
index 10ed6ffbc2..018ec014e6 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/sprm/SectionSprmUncompressor.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/sprm/SectionSprmUncompressor.java
@@ -46,7 +46,14 @@ public final class SectionSprmUncompressor extends SprmUncompressor {
while (sprmIt.hasNext())
{
SprmOperation sprm = sprmIt.next();
- unCompressSEPOperation(newProperties, sprm);
+ try
+ {
+ unCompressSEPOperation(newProperties, sprm);
+ }
+ catch (Exception exc)
+ {
+ LOG.atError().withThrowable(exc).log("Unable to apply SPRM operation '{}'", box(sprm.getOperation()));
+ }
}
return newProperties;
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/sprm/SprmOperation.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/sprm/SprmOperation.java
index 705de1f1c9..cd1984c5f7 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/sprm/SprmOperation.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/sprm/SprmOperation.java
@@ -19,12 +19,16 @@ package org.apache.poi.hwpf.sprm;
import java.util.Arrays;
+import org.apache.logging.log4j.Logger;
+import org.apache.poi.logging.PoiLogManager;
import org.apache.poi.util.BitField;
import org.apache.poi.util.BitFieldFactory;
import org.apache.poi.util.Internal;
import org.apache.poi.util.LittleEndian;
import org.apache.poi.util.LittleEndianConsts;
+import static org.apache.logging.log4j.util.Unbox.box;
+
/**
* This class is used to represent a sprm operation from a Word 97/2000/XP
* document.
@@ -32,6 +36,8 @@ import org.apache.poi.util.LittleEndianConsts;
@Internal(since="3.8 beta 4")
public final class SprmOperation
{
+ private static final Logger LOG = PoiLogManager.getLogger(SprmOperation.class);
+
private static final BitField BITFIELD_OP = BitFieldFactory
.getInstance( 0x1ff );
private static final BitField BITFIELD_SIZECODE = BitFieldFactory
@@ -104,13 +110,18 @@ public final class SprmOperation
case 3:
return LittleEndian.getInt( _grpprl, _gOffset );
case 6:
+ if ( _gOffset + 1 >= _grpprl.length )
+ {
+ // truncated SPRM, no operand present
+ return 0;
+ }
// surely shorter than an int...
- byte operandLength = _grpprl[_gOffset + 1];
+ int operandLength = Math.min( _grpprl[_gOffset + 1], LittleEndianConsts.INT_SIZE );
// initialized to zeros by JVM
byte[] codeBytes = new byte[LittleEndianConsts.INT_SIZE];
for ( int i = 0; i < operandLength; i++ )
- if ( _gOffset + i < _grpprl.length )
+ if ( _gOffset + 1 + i < _grpprl.length )
codeBytes[i] = _grpprl[_gOffset + 1 + i];
return LittleEndian.getInt( codeBytes, 0 );
@@ -169,11 +180,19 @@ public final class SprmOperation
int offset = _gOffset;
if ( sprm == SPRM_LONG_TABLE || sprm == SPRM_LONG_PARAGRAPH )
{
+ if ( offset + LittleEndianConsts.SHORT_SIZE > _grpprl.length )
+ {
+ return truncatedSize( sprm );
+ }
int retVal = ( 0x0000ffff &
LittleEndian.getShort( _grpprl, offset ) ) + 3;
_gOffset += 2;
return retVal;
}
+ if ( offset >= _grpprl.length )
+ {
+ return truncatedSize( sprm );
+ }
return ( 0x000000ff & _grpprl[_gOffset++] ) + 3;
case 7:
return 5;
@@ -183,6 +202,19 @@ public final class SprmOperation
}
}
+ /**
+ * A variable-length SPRM whose operand-length byte(s) lie beyond the end
+ * of the grpprl: the buffer is truncated (or padded with junk), so treat
+ * whatever is left as the size of this SPRM rather than throwing an
+ * {@link ArrayIndexOutOfBoundsException}. The iterator then stops here.
+ */
+ private int truncatedSize( short sprm )
+ {
+ LOG.atWarn().log( "SPRM 0x{} at offset {} is truncated, grpprl length is {}",
+ Integer.toHexString( sprm & 0xffff ), box(_offset), box(_grpprl.length) );
+ return _grpprl.length - _offset;
+ }
+
public int size()
{
return _size;
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/sprm/TableSprmUncompressor.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/sprm/TableSprmUncompressor.java
index 5cf224f059..c23d558a13 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/sprm/TableSprmUncompressor.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/sprm/TableSprmUncompressor.java
@@ -41,10 +41,9 @@ public final class TableSprmUncompressor extends SprmUncompressor {
}
SprmOperation sprmOperation = sprmBuffer.findSprm( (short) 0xd608 );
- if ( sprmOperation != null ) {
- byte[] grpprl = sprmOperation.getGrpprl();
- int offset = sprmOperation.getGrpprlOffset();
- short itcMac = grpprl[offset];
+ byte[] grpprl = sprmOperation == null ? null : sprmOperation.getGrpprl();
+ if ( grpprl != null && sprmOperation.getGrpprlOffset() < grpprl.length ) {
+ short itcMac = grpprl[sprmOperation.getGrpprlOffset()];
tableProperties = new TableProperties( itcMac );
} else {
LOG.atWarn().log("Some table rows didn't specify number of columns in SPRMs");
diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hwpf/sprm/TestSprmOperation.java b/poi-scratchpad/src/test/java/org/apache/poi/hwpf/sprm/TestSprmOperation.java
new file mode 100644
index 0000000000..64122a0401
--- /dev/null
+++ b/poi-scratchpad/src/test/java/org/apache/poi/hwpf/sprm/TestSprmOperation.java
@@ -0,0 +1,126 @@
+/* ====================================================================
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hwpf.sprm;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.poi.hwpf.usermodel.ParagraphProperties;
+import org.apache.poi.hwpf.usermodel.SectionProperties;
+import org.apache.poi.hwpf.usermodel.TableProperties;
+import org.junit.jupiter.api.Test;
+
+class TestSprmOperation {
+
+ /**
+ * Bug 66245: a variable-length SPRM (size code 6) at the very end of a
+ * grpprl, with no room for its operand-length byte, must not throw an
+ * {@link ArrayIndexOutOfBoundsException}. It consumes the remaining bytes
+ * so that the iterator terminates.
+ */
+ @Test
+ void truncatedVariableLengthSprm() {
+ // sprmPJc (0x2403, 1 byte operand) followed by a size-code-6 PAP sprm
+ // opcode (0xC61D) with no length byte
+ byte[] grpprl = { 0x03, 0x24, 0x01, 0x1D, (byte) 0xC6 };
+
+ SprmIterator it = new SprmIterator(grpprl, 0);
+ assertTrue(it.hasNext());
+ SprmOperation first = it.next();
+ assertEquals(3, first.size());
+ assertEquals(1, first.getOperand());
+
+ assertTrue(it.hasNext());
+ SprmOperation truncated = it.next();
+ assertEquals(6, truncated.getSizeCode());
+ assertEquals(2, truncated.size());
+ assertFalse(it.hasNext());
+
+ ParagraphProperties pap = ParagraphSprmUncompressor.uncompressPAP(
+ new ParagraphProperties(), grpprl, 0);
+ assertNotNull(pap);
+ assertEquals(1, pap.getJc());
+ }
+
+ /**
+ * Same as above for sprmPChgTabs (0xC615), whose operand length is a
+ * two-byte value: only one of the two bytes is present.
+ */
+ @Test
+ void truncatedLongParagraphSprm() {
+ byte[] grpprl = { 0x03, 0x24, 0x01, 0x15, (byte) 0xC6, 0x10 };
+
+ SprmIterator it = new SprmIterator(grpprl, 0);
+ assertEquals(3, it.next().size());
+ assertTrue(it.hasNext());
+ SprmOperation truncated = it.next();
+ assertEquals(3, truncated.size());
+ assertFalse(it.hasNext());
+
+ ParagraphProperties pap = ParagraphSprmUncompressor.uncompressPAP(
+ new ParagraphProperties(), grpprl, 0);
+ assertEquals(1, pap.getJc());
+ }
+
+ /**
+ * The operand of a variable-length SPRM must not be read past the end of
+ * the grpprl, nor past the 4 bytes that fit into an int.
+ */
+ @Test
+ void variableLengthOperandBounds() {
+ // length byte says 2, but the "operand length" the code reads from the
+ // second operand byte claims 0x7F bytes: only what is there is used
+ byte[] grpprl = { 0x1D, (byte) 0xC6, 0x02, 0x05, 0x7F };
+ SprmOperation sprm = new SprmOperation(grpprl, 0);
+ assertEquals(5, sprm.size());
+ assertEquals(0x7F, sprm.getOperand());
+
+ // no operand at all
+ SprmOperation truncated = new SprmOperation(new byte[] { 0x1D, (byte) 0xC6 }, 0);
+ assertEquals(2, truncated.size());
+ assertEquals(0, truncated.getOperand());
+ assertNotNull(truncated.toString());
+ }
+
+ /**
+ * A SEPX whose last SPRM is missing part of its operand: the preceding
+ * SPRMs are still applied, the broken one is logged and skipped.
+ */
+ @Test
+ void truncatedSectionSprm() {
+ // sprmSCnsPgn (0x3000) = 1, then a 2-byte-operand SEP sprm (0x5001)
+ // with only one operand byte
+ byte[] grpprl = { 0x00, 0x30, 0x01, 0x01, 0x50, 0x02 };
+ SectionProperties sep = SectionSprmUncompressor.uncompressSEP(grpprl, 0);
+ assertEquals(1, sep.getCnsPgn());
+ }
+
+ /**
+ * A TAPX whose sprmTDefTable (0xD608) declares an operand that is not
+ * actually present.
+ */
+ @Test
+ void truncatedTableDefinitionSprm() {
+ byte[] grpprl = { 0x00, 0x00, 0x08, (byte) 0xD6, 0x01, 0x00 };
+ TableProperties tap = TableSprmUncompressor.uncompressTAP(new SprmBuffer(grpprl, 2));
+ assertNotNull(tap);
+ assertEquals(1, tap.getItcMac());
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org