Avoid OOM with incorrect property sizes

Add allocation check to verify size >= 0 and < 1mio
Also reformat code to match general coding style

Fixes https://issues.oss-fuzz.com/issues/485091380
This commit is contained in:
Dominik Stadler 2026-02-21 14:29:08 +01:00
parent 23369586da
commit 44598bd030
7 changed files with 32 additions and 42 deletions

View File

@ -33,6 +33,7 @@ public class TestSlideIdListing extends BaseTestPPTIterating {
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-5306877435838464.ppt");
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6360479850954752.ppt");
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6028723156746240.ppt");
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-4983252485210112.ppt");
}
@Test

View File

@ -33,6 +33,7 @@ public class TestSlideShowRecordDumper extends BaseTestPPTIterating {
static {
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6360479850954752.ppt");
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6028723156746240.ppt");
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-4983252485210112.ppt");
}
@Test

View File

@ -32,6 +32,7 @@ public class TestUserEditAndPersistListing extends BaseTestPPTIterating {
static {
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6360479850954752.ppt");
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6028723156746240.ppt");
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-4983252485210112.ppt");
}
@Test

View File

@ -23,6 +23,7 @@ import java.util.Map;
import java.util.function.Supplier;
import org.apache.poi.util.GenericRecordUtil;
import org.apache.poi.util.IOUtils;
import org.apache.poi.util.LittleEndian;
/**
@ -30,6 +31,9 @@ import org.apache.poi.util.LittleEndian;
* {@link EscherTertiaryOptRecord}
*/
public abstract class AbstractEscherOptRecord extends EscherRecord {
// arbitrary limit, can be adjusted if it turns out to be too low
private static final int MAX_PROPERTY_SIZE = 1_000_000;
private final List<EscherProperty> properties = new ArrayList<>();
protected AbstractEscherOptRecord() {}
@ -39,21 +43,17 @@ public abstract class AbstractEscherOptRecord extends EscherRecord {
properties.addAll(other.properties);
}
/**
* Add a property to this record.
*
* @param prop the escher property to add
*/
public void addEscherProperty( EscherProperty prop )
{
public void addEscherProperty( EscherProperty prop ) {
properties.add( prop );
}
@Override
public int fillFields( byte[] data, int offset,
EscherRecordFactory recordFactory )
{
public int fillFields( byte[] data, int offset, EscherRecordFactory recordFactory ) {
int bytesRemaining = readHeader( data, offset );
if (bytesRemaining < 0) {
throw new IllegalStateException("Invalid value for bytesRemaining: " + bytesRemaining);
@ -72,8 +72,7 @@ public abstract class AbstractEscherOptRecord extends EscherRecord {
*
* @return the list of properties
*/
public List<EscherProperty> getEscherProperties()
{
public List<EscherProperty> getEscherProperties() {
return properties;
}
@ -83,26 +82,23 @@ public abstract class AbstractEscherOptRecord extends EscherRecord {
* @param index the ordinal index of the property
* @return the escher property
*/
public EscherProperty getEscherProperty( int index )
{
public EscherProperty getEscherProperty( int index ) {
return properties.get( index );
}
private int getPropertiesSize()
{
private int getPropertiesSize() {
int totalSize = 0;
for ( EscherProperty property : properties )
{
totalSize += property.getPropertySize();
for ( EscherProperty property : properties ) {
int propertySize = property.getPropertySize();
IOUtils.safelyAllocateCheck(propertySize, MAX_PROPERTY_SIZE);
totalSize += propertySize;
}
return totalSize;
}
@Override
public int getRecordSize()
{
public int getRecordSize() {
return 8 + getPropertiesSize();
}
@ -116,9 +112,7 @@ public abstract class AbstractEscherOptRecord extends EscherRecord {
}
@Override
public int serialize( int offset, byte[] data,
EscherSerializationListener listener )
{
public int serialize( int offset, byte[] data, EscherSerializationListener listener ) {
listener.beforeRecordSerialize( offset, getRecordId(), this );
LittleEndian.putShort( data, offset, getOptions() );

View File

@ -70,8 +70,7 @@ public abstract class EscherRecord implements Duplicatable, GenericRecord {
*
* @see #fillFields(byte[], int, org.apache.poi.ddf.EscherRecordFactory)
*/
protected int fillFields( byte[] data, EscherRecordFactory f )
{
protected int fillFields( byte[] data, EscherRecordFactory f ) {
return fillFields( data, 0, f );
}
@ -154,8 +153,7 @@ public abstract class EscherRecord implements Duplicatable, GenericRecord {
* @return The options field for this record. All records have one.
*/
@Internal
public short getOptions()
{
public short getOptions() {
return _options;
}
@ -183,8 +181,7 @@ public abstract class EscherRecord implements Duplicatable, GenericRecord {
* @return the serialized record.
* @see #serialize(int, byte[])
*/
public byte[] serialize()
{
public byte[] serialize() {
byte[] retval = new byte[getRecordSize()];
serialize( 0, retval );
@ -201,8 +198,7 @@ public abstract class EscherRecord implements Duplicatable, GenericRecord {
*
* @see #serialize(int, byte[], org.apache.poi.ddf.EscherSerializationListener)
*/
public int serialize( int offset, byte[] data)
{
public int serialize( int offset, byte[] data) {
return serialize( offset, data, new NullEscherSerializationListener() );
}
@ -252,7 +248,9 @@ public abstract class EscherRecord implements Duplicatable, GenericRecord {
*
* @see EscherContainerRecord
*/
public List<EscherRecord> getChildRecords() { return Collections.emptyList(); }
public List<EscherRecord> getChildRecords() {
return Collections.emptyList();
}
/**
* Sets the child records for this record. By default this will throw
@ -281,8 +279,7 @@ public abstract class EscherRecord implements Duplicatable, GenericRecord {
* @param w The print writer to output to.
* @param indent The current indent level.
*/
public void display(PrintWriter w, int indent)
{
public void display(PrintWriter w, int indent) {
for (int i = 0; i < indent * 4; i++) {
w.print(' ');
}
@ -301,8 +298,7 @@ public abstract class EscherRecord implements Duplicatable, GenericRecord {
*
* @return The instance part of the record
*/
public short getInstance()
{
public short getInstance() {
return fInstance.getShortValue( _options );
}
@ -311,8 +307,7 @@ public abstract class EscherRecord implements Duplicatable, GenericRecord {
*
* @param value instance part value
*/
public void setInstance( short value )
{
public void setInstance( short value ) {
_options = fInstance.setShortValue( _options, value );
}
@ -321,8 +316,7 @@ public abstract class EscherRecord implements Duplicatable, GenericRecord {
*
* @return The version part of the option record
*/
public short getVersion()
{
public short getVersion() {
return fVersion.getShortValue( _options );
}
@ -331,12 +325,11 @@ public abstract class EscherRecord implements Duplicatable, GenericRecord {
*
* @param value version part value
*/
public void setVersion( short value )
{
public void setVersion( short value ) {
_options = fVersion.setShortValue( _options, value );
}
public String toXml(){
public String toXml() {
return toXml("");
}
@ -344,7 +337,7 @@ public abstract class EscherRecord implements Duplicatable, GenericRecord {
* @param tab - each children must be indented right relative to its parent
* @return xml representation of this record
*/
public final String toXml(String tab){
public final String toXml(String tab) {
return GenericRecordXmlWriter.marshal(this);
}

Binary file not shown.