Skip to content

Commit 145ca46

Browse files
Harden HPSF allocation-size arithmetic against silent overflow (#1064)
* Harden HPSF allocation-size arithmetic against silent overflow * updated * updated * Revert "updated" This reverts commit e137e3b. * use RecordFormatException * Update poi-integration-exceptions.csv --------- Co-authored-by: PJ Fanning <pjfanning@users.noreply.github.com>
1 parent c0a49d5 commit 145ca46

4 files changed

Lines changed: 102 additions & 3 deletions

File tree

poi/src/main/java/org/apache/poi/hpsf/Array.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
1919
import org.apache.poi.util.IOUtils;
2020
import org.apache.poi.util.Internal;
2121
import org.apache.poi.util.LittleEndianByteArrayInputStream;
22+
import org.apache.poi.util.RecordFormatException;
2223

2324
@Internal
2425
public class Array {
@@ -71,7 +72,14 @@ void read( LittleEndianByteArrayInputStream lei ) {
7172
long getNumberOfScalarValues() {
7273
long result = 1;
7374
for ( ArrayDimension dimension : _dimensions ) {
74-
result *= dimension._size;
75+
try {
76+
// Math.multiplyExact rejects compounding dimensions that would silently
77+
// overflow long math and bypass the > Integer.MAX_VALUE guard in Array.read.
78+
result = Math.multiplyExact(result, dimension._size);
79+
} catch (ArithmeticException e) {
80+
throw new RecordFormatException(
81+
"Overflow when calculating the number of scalar values", e);
82+
}
7583
}
7684
return result;
7785
}

poi/src/main/java/org/apache/poi/hpsf/UnicodeString.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
2828
import org.apache.poi.util.LittleEndian;
2929
import org.apache.poi.util.LittleEndianByteArrayInputStream;
3030
import org.apache.poi.util.LittleEndianConsts;
31+
import org.apache.poi.util.RecordFormatException;
3132
import org.apache.poi.util.StringUtil;
3233

3334
@Internal
@@ -36,9 +37,20 @@ public class UnicodeString {
3637

3738
private byte[] _value;
3839

40+
/**
41+
* @param lei a LittleEndianByteArrayInputStream
42+
* @throws RecordFormatException if there is a problem with calculated length
43+
*/
3944
public void read(LittleEndianByteArrayInputStream lei) {
4045
final int length = lei.readInt();
41-
final int unicodeBytes = length*2;
46+
// Math.multiplyExact rejects crafted lengths that would silently wrap signed-int math
47+
// (e.g. length == 0x40000001 -> length*2 == 0x80000002 wraps to negative).
48+
final int unicodeBytes;
49+
try {
50+
unicodeBytes = Math.multiplyExact(length, 2);
51+
} catch (ArithmeticException e) {
52+
throw new RecordFormatException("Invalid unicode length", e);
53+
}
4254
_value = IOUtils.safelyAllocate(unicodeBytes, CodePageString.getMaxRecordLength());
4355

4456
// If Length is zero, this field MUST be zero bytes in length. If Length is
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
/* ====================================================================
2+
Licensed to the Apache Software Foundation (ASF) under one or more
3+
contributor license agreements. See the NOTICE file distributed with
4+
this work for additional information regarding copyright ownership.
5+
The ASF licenses this file to You under the Apache License, Version 2.0
6+
(the "License"); you may not use this file except in compliance with
7+
the License. You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
==================================================================== */
17+
package org.apache.poi.hpsf;
18+
19+
import static org.junit.jupiter.api.Assertions.assertThrows;
20+
21+
import org.apache.poi.util.LittleEndian;
22+
import org.apache.poi.util.LittleEndianByteArrayInputStream;
23+
import org.apache.poi.util.RecordFormatException;
24+
import org.junit.jupiter.api.Test;
25+
26+
/**
27+
* Verifies that HPSF readers reject crafted lengths whose subsequent
28+
* allocation-size arithmetic would silently overflow signed integer math.
29+
*/
30+
class TestOverflowHardening {
31+
32+
/**
33+
* Reproduces the int*2 multiplication overflow in {@link UnicodeString#read}:
34+
* a length of {@code 0x40000001} produces {@code length*2 == 0x80000002},
35+
* which wraps to a negative int. {@link Math#multiplyExact} now surfaces
36+
* the overflow as an {@link ArithmeticException} at the parser, where the
37+
* malformed field actually lives.
38+
*/
39+
@Test
40+
void unicodeStringLengthMultiplicationOverflowRejected() {
41+
byte[] data = new byte[4];
42+
LittleEndian.putInt(data, 0, 0x40000001);
43+
LittleEndianByteArrayInputStream lei = new LittleEndianByteArrayInputStream(data, 0);
44+
45+
UnicodeString us = new UnicodeString();
46+
assertThrows(RecordFormatException.class, () -> us.read(lei));
47+
}
48+
49+
/**
50+
* Reproduces the long*long multiplication overflow in
51+
* {@link Array.ArrayHeader#getNumberOfScalarValues}: with three dimensions
52+
* of size {@code 0x80000000} the unchecked product
53+
* {@code 2^31 * 2^31 * 2^31 = 2^93} wraps inside a 64-bit long and the
54+
* subsequent {@code > Integer.MAX_VALUE} guard at {@code Array.read} is
55+
* silently bypassed. {@link Math#multiplyExact} now rejects the crafted
56+
* array header with an {@link ArithmeticException}.
57+
*/
58+
@Test
59+
void arrayDimensionMultiplicationOverflowRejected() {
60+
// ArrayHeader layout: type (4) + numDimensions (4) + numDimensions * (size:4 + indexOffset:4)
61+
// 3 dimensions of size 0x80000000 each -> product overflows 2^63 (long max)
62+
byte[] data = new byte[4 + 4 + 3 * (4 + 4)];
63+
int off = 0;
64+
LittleEndian.putInt(data, off, Variant.VT_I4);
65+
off += 4;
66+
LittleEndian.putInt(data, off, 3);
67+
off += 4;
68+
for (int i = 0; i < 3; i++) {
69+
LittleEndian.putUInt(data, off, 0x80000000L);
70+
off += 4;
71+
LittleEndian.putInt(data, off, 0);
72+
off += 4;
73+
}
74+
LittleEndianByteArrayInputStream lei = new LittleEndianByteArrayInputStream(data, 0);
75+
76+
Array a = new Array();
77+
assertThrows(RecordFormatException.class, () -> a.read(lei));
78+
}
79+
}

test-data/poi-integration-exceptions.csv

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ spreadsheet/sample.strict.xlsx,extract,OPC,,org.apache.poi.ooxml.POIXMLException
100100
spreadsheet/57914.xlsx,"handle,extract",XSSF,,org.apache.poi.ooxml.POIXMLException,"Strict OOXML isn't currently supported, please see bug #57699",
101101
spreadsheet/57914.xlsx,extract,OPC,,org.apache.poi.ooxml.POIXMLException,"Strict OOXML isn't currently supported, please see bug #57699",
102102
spreadsheet/poi-fuzz.xls,additional,HSSF,,org.apache.poi.util.RecordFormatException,Not enough data (0) to read requested (4) bytes,
103-
spreadsheet/poi-fuzz.xls,additional,HPSF,,org.apache.poi.util.RecordFormatException,"Tried to allocate an array of length 1,468,570,764, but the maximum length for this record type is 100,000",
103+
spreadsheet/poi-fuzz.xls,additional,HPSF,,org.apache.poi.util.RecordFormatException,Overflow when calculating the number of scalar values,
104104
spreadsheet/poi-fuzz.xls,handle,HPSF,,org.opentest4j.AssertionFailedError,expected: <true> but was: <false>,
105105
openxml4j/ContentTypeHasEntities.ooxml,"handle,extract",OPC,,org.apache.poi.openxml4j.exceptions.InvalidFormatException,Can't read content types part !,
106106
ddf/Container.dat,handle,HMEF,,java.lang.IllegalArgumentException,"TNEF signature not detected in file, expected 574529400 but got -268435441",

0 commit comments

Comments
 (0)