From 44598bd03056e3415357c8e49a614fbd8c75c2e6 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sat, 21 Feb 2026 14:29:08 +0100 Subject: [PATCH] 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 --- .../poi/hslf/dev/TestSlideIdListing.java | 1 + .../hslf/dev/TestSlideShowRecordDumper.java | 1 + .../dev/TestUserEditAndPersistListing.java | 1 + .../poi/ddf/AbstractEscherOptRecord.java | 36 ++++++++---------- .../java/org/apache/poi/ddf/EscherRecord.java | 35 +++++++---------- ...nimized-POIHSLFFuzzer-4983252485210112.ppt | Bin 0 -> 8960 bytes test-data/spreadsheet/stress.xls | Bin 80896 -> 81408 bytes 7 files changed, 32 insertions(+), 42 deletions(-) create mode 100644 test-data/slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-4983252485210112.ppt diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideIdListing.java b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideIdListing.java index 684f4e8e42..9727196a73 100644 --- a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideIdListing.java +++ b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideIdListing.java @@ -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 diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowRecordDumper.java b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowRecordDumper.java index f5705fe981..b2e3bc373e 100644 --- a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowRecordDumper.java +++ b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowRecordDumper.java @@ -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 diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestUserEditAndPersistListing.java b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestUserEditAndPersistListing.java index e639e292b0..a9362c7b42 100644 --- a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestUserEditAndPersistListing.java +++ b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestUserEditAndPersistListing.java @@ -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 diff --git a/poi/src/main/java/org/apache/poi/ddf/AbstractEscherOptRecord.java b/poi/src/main/java/org/apache/poi/ddf/AbstractEscherOptRecord.java index 536e590a2d..f10f6f24f1 100644 --- a/poi/src/main/java/org/apache/poi/ddf/AbstractEscherOptRecord.java +++ b/poi/src/main/java/org/apache/poi/ddf/AbstractEscherOptRecord.java @@ -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 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 getEscherProperties() - { + public List 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() ); diff --git a/poi/src/main/java/org/apache/poi/ddf/EscherRecord.java b/poi/src/main/java/org/apache/poi/ddf/EscherRecord.java index de5b95b22d..4ed0974d99 100644 --- a/poi/src/main/java/org/apache/poi/ddf/EscherRecord.java +++ b/poi/src/main/java/org/apache/poi/ddf/EscherRecord.java @@ -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 getChildRecords() { return Collections.emptyList(); } + public List 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); } diff --git a/test-data/slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-4983252485210112.ppt b/test-data/slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-4983252485210112.ppt new file mode 100644 index 0000000000000000000000000000000000000000..0bec934f7e763bdd59011caed8452f5244731556 GIT binary patch literal 8960 zcmeHM3vg7`8UF9x4Z8utT@p|RP%jScfYlgl9}Km)2%7<0V4H`nV|c8|hO8u6XLs>I zt67S*6f}yoDn1ZjtuofBcB-RO8>dRE4pXe1T4(w|M?1yGn5|j_NjUw!bM`JbA(5cg z(Wxi7=ltjS|K~sd>zsXd*Z3E2e_--UGAPZFfE-liGRoI(`Cy5NIH)9_5JBF;Z4Ka~@~p>a{I5FWEKz%GYGOJTh~S zf1&u!dmn22>F=xxH0DEETqF+c7fTX&38gd*uFMDAEUTphJ)~;*G(&95Mt1-oSu^~| zLp>bdS7qvDQm^rJNJ8&@A@H>Vdj_}@av>;6qhnY=dwoB8 za);4V1NN&r0fG7Iue*A=C z8cO^a-1lj&_k5a(ABJ{E_5^Oi!CUfbUt&P+6?$_*H_6?&y1~M$;5fNd#i-sJ5}A% zhxX9ZV{2G4aV;a6cR7^DyhPcIbhpL}AY3gr5IxS*lcw0kgnnW?lrFvd*HI`?NCkOrJ03=$Z{j&nt30P7k}Q>D(;Nv42( zD+2wt>Lo8s+TO3G#*S-x`!Y?mndv*>;wXB8=%p$3^-Z#g4km>53G+*|LXyh_y6B74#NOJ*=a2Oxe;{~8n~YgtQ51EuN8Rwj zU36400T)Se*KT^ZvJ4+6_a?DEC31`5;Ar`~5wv%EH3^T2eD3{SwC57rMbaxz=aKd7 z&Ls8HA$g8yN-moEjU;tcK;JowYC2_#Ots$4Pw2w_|njB?}ifxbv6H zYg`mt+#r8KZ%3kIVBgD+-Kl%~-c7p@fitW(8U+dJG-$iR5#q-EE3Nc~4iV;Q*kXQjNeAQPHz_!O>h@x|GUxnXVRxwIwpk zFPffMnLQLCDp4mG>7F{@TMUdX+J?v_db-GRr4AH9OJ99z57{TBqL_)^o4P<#k6vXS z0@MutOepM1qhxi!46!XPQz}MoVXfoLWU@?cQGHlq1U8n4ehEWsUqegYucT(4lp)p+ z{5Jf^IWlvPE3ybf=7@D(StHbi+EU-RXwlrda~CdNQnzSs!@?ztBM@l4v7s`|j^e|B z^M>jkdD|f?uM(=cEu;FEx3=TB)G0Msp2mej=GP^PG+hplnzoi!f>#w-0WAm&Yw1um z4aaQKx@k0&W0)F5{`wwbnpF0jwDDw6gRtEHHaiC|JC2QqQWW6KL)C#0$@*la5xZrQ zJyMbJ4DA0_WIRTGl#b`i6bd??Q@Hxa;&_ZI5Zzv5rvPzFC*#Y*QjRLd2h&l{!5;ID zZ%juDExu$MJ7VM+?UZw!3ZI4W8FOj-tnDJ5yq?EJ(@%lla@N{KHR8DK#5 zZggPvO@RJj#20McJd!Jg^4}?jy#Fki$AIFxjk~vBbyIg4jVr?3pivn&@1T|Cb%PB z4C%era&UN^1@9H!Y<1tZQGRUw{=KP)_m>ryK_BT75VDk24hmcqiC0Q9EV%HSYbWYVT~x zj?)0jpa{q(P3%2#!X49pa)t>D-1E)H$-V~&oFI4`4MvT%#-tU)KR|fG z$npYzMZyw-89?*z@TIW_E6i1gtv`AhfCs^AiI<_O@ z3sek|C&uusnJQ!x+A9h;cciT)!KYYnMT;la4x-MXFt@c1#vJxx?hO$0IU!;`1FJbQ zAwnN#$-lKQZP~s3pq3uAF;M}qJ?x>P#-oy~A2$S*Y#*$AA;!jsi|~-jhml7q(X)s# zkxeB!t6S5Zx$0DBOY#y~j=wnm?o)qIyKo=eSY>$pdNp}`{WCkPh;`-+k<76ZP`4zC zT;z@*UbjA+YWXe~ddVYZ|EH=2DfuvOdyiAh3eGCZo4g(1A?oK2lv=TMJNt}npw*yl z;G_FcCg#eIz{J3k;IWh{>xmO_UWm3i@-^+|Stor{sY^Q9GRe@Z+b8(;P$~V!ouR)H z#)PYw+m@b~=S;3bv9@J`ZN+Zag?^9C#)J;d{D2 zsGnOcDKDBP4Dcnp=vBEmhGSl^@H|S-EZJyhK(QGH@hbqkKxrBFaP5+A`Jtw|_%O>2 zgnQt;#>?Y$VPQUQmE)lcX9&WKq5L_Xj#o?!tShKDodPC!CWw$F-N0T^_zo+@v(_Na z+SMit*_GSUZn7nna~CDDxn#!8rE+jF+1#b=NvOQT-A^-<;3{~jWX;4C<(ezO!ekKV z?M}a~V0T_D;qC6pY`6E|jM2mIYrk>|VV?&c>;O2~F7E3Nz}~*tP8qI4KCeC1yvofD z+85s7%(k)>^>|Z)A#8UqO14)tw59OA)Se_S@i*qbLB53Mja4VISMJkMqXv;h@fn_a zEtI5IT`^XJ+Ys62Oho~EfFMyDaA3|T2Ill<`#0#TLV9{O9Se@LeC@e#1NuofM{J2Z@nO!p0 zVhsuG*3KUw_jy!5cKd#e+Be|GD7~^QLol3|sHq(7k5T&af}cD18j7&oHa|+~wW6Bz z5&amY%@<<9+EgZ+bL-nvU~qWHTr8s)c!yz$ybzD&asEmP{$M`{u#=_9kQc*`4;npJ7$#VNG+T2qz*hP0RU$5;pBoRs{a>)VxRA%h6CFwTM3TD~4 v*ZDkJDW=OG-}LeexIT=NcLH>$u}pG}8-uxNtgFLAq^kq0Y-$bcW-0v_Zp6~* literal 0 HcmV?d00001 diff --git a/test-data/spreadsheet/stress.xls b/test-data/spreadsheet/stress.xls index ed7f4227220839c6a9f436ca5ad6fc83ebfab24a..ac5b024338c349e3a536e2083852d9b32f1adeca 100644 GIT binary patch delta 2712 zcmZ9MU2Kz87{|}kwWaI2z1^VO*cj_z8x;1@b}MaH2ZOFj?XWC|6t<-HmgRg_xjREqZ5Sb6p11#PIc>8(JHPiizyEp9 z`_X^b+5f=#W;pj9g^ypUv%}4OzT(M)LcD6-u|3Zn?%jXr;L+EVUoG+ve(rn2Rp(Kv zeWW{g?=HGE^67z28XVr(6lhE)8ltgiVq?-QmN%ZG*{GODbxDyZd^B`+D{U zYa_9EG7<|Xl96D~!M(vq*#3+Z9}2l#XF2ogNLz0g<&SL%*eiNP`4sVYeDH(5^3`M6 zv(%}aYOHE@>{2c@cC2t_mRq^ic**SXIZDy^i2uc|lW#d_SKtgcKAf7Gn*KW{G5@K}em-_nmoK1Fx}j5ClE1>@F)SX# z;xR1Qw&gc{I&B#e%Q!4v!{Rk8Uc-`OTPE~W+EQA^mfvBK+pTEkqWKJq&#>g$mOu0% zp3Y};ms-0<_el zl7>9G#P6jDZ>8Me)3nNSEb=-bBNA4ti_-$XuY|d zTPp!qGptm3I#9!xwF+X2)Wv+*yhXqy*;G=-21l9sR26$1T>Hs$wQ5!O%Hri43 z=-miGejQuGfSUlD1x5fL2i(sv6$R`tqB=67Hrr7R=-mv#WwAB_ZULMS7z2C)aDF{U ztpMC=L~YH8+Ga<^(YpNpA^!+(MlDxcR7-^~^y<9HM=68IrBo^;*#vd{$acXp!VSbpnt;tI72tC%ic zIkpL#m>$hoUS$_{F`K<|K3)i#oqX?D|7&NgB+poAIB8*VaWVFLJNkl!-k>)RTcgrS z=iIU{oo1p}GN^@|lFNx+!~esf@mr$JO{weyGOx@`f5eNP8ZY<^70tc70tvuU8hku(0Kp?9QVc`bj>= zBY2CGQ+zI7#EESFTJ6YJMQI9cG#I8t#i!+}se@GvX<-NEJ z7JeSyY#H*5ih&dNRVS{B+5%_IOGfu z*BZ;x`&Xu4sV21&%5xwM;41y*q##=bK|v`Orvg?3Zc{J~um*6HVJIE2)=*Vzs;bkgGLWr<;F_{#0@ef0 zE0_h?09fkfse?SLS{0+$zXE8t}X zeSnVwURPCRfSrb_PE%EvUbO+)E(rXUY%K@e1~{Z(1>j?VCmDt+0lN)V-KMJTdX*p9 z?GW5l)+)dqfSR9SHDC{5fMKWxaHpYar>Sa}UR8_iE(lI5YaQTjz^e+@13nITTU9jx z_8O{sO;vmJssOTkAP85nH3--T_`HIRfc=1H8HPfD1BR*rQ`Mke)r9OI1b39R8E^Nbz$XB=G7Pl