From d1f3f9489fc0f2fc32b109d39817ec29067ab127 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sun, 18 Jan 2026 17:38:48 +0100 Subject: [PATCH] Add check for too large allocation in SharedFormulaGroup A malformed spreadsheet could trigger a very large allocation. Can be overruled by users via IOUtils.setByteArrayMaxOverride(). Fixes https://issues.oss-fuzz.com/issues/476431391 --- .../record/aggregates/SharedValueManager.java | 9 ++++++++- .../java/org/apache/poi/util/IOUtils.java | 8 ++++---- .../poi/hssf/dev/TestBiffDrawingToXml.java | 2 ++ .../apache/poi/hssf/dev/TestBiffViewer.java | 1 + .../poi/hssf/dev/TestFormulaViewer.java | 1 + .../apache/poi/hssf/dev/TestRecordLister.java | 1 + ...nimized-POIHSSFFuzzer-4734163573080064.xls | Bin 0 -> 7168 bytes test-data/spreadsheet/stress.xls | Bin 77312 -> 77312 bytes 8 files changed, 17 insertions(+), 5 deletions(-) create mode 100644 test-data/spreadsheet/clusterfuzz-testcase-minimized-POIHSSFFuzzer-4734163573080064.xls diff --git a/poi/src/main/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java b/poi/src/main/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java index 0a22a74f67..44763597cb 100644 --- a/poi/src/main/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java +++ b/poi/src/main/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java @@ -31,6 +31,7 @@ import org.apache.poi.hssf.record.TableRecord; import org.apache.poi.ss.formula.ptg.ExpPtg; import org.apache.poi.hssf.util.CellRangeAddress8Bit; import org.apache.poi.ss.util.CellReference; +import org.apache.poi.util.IOUtils; /** * Manages various auxiliary records while constructing a @@ -42,6 +43,7 @@ import org.apache.poi.ss.util.CellReference; * */ public final class SharedValueManager { + private static final int MAX_NUMBER_AGGS = 10_000; private static final class SharedFormulaGroup { private final SharedFormulaRecord _sfr; @@ -63,7 +65,12 @@ public final class SharedValueManager { _firstCell = firstCell; int width = sfr.getLastColumn() - sfr.getFirstColumn() + 1; int height = sfr.getLastRow() - sfr.getFirstRow() + 1; - _frAggs = new FormulaRecordAggregate[width * height]; + + // ensure we do not try to initialize a very large amount of formula-record-aggregates + int allocateSize = width * height; + IOUtils.safelyAllocateCheck(allocateSize, MAX_NUMBER_AGGS); + + _frAggs = new FormulaRecordAggregate[allocateSize]; _numberOfFormulas = 0; } diff --git a/poi/src/main/java/org/apache/poi/util/IOUtils.java b/poi/src/main/java/org/apache/poi/util/IOUtils.java index 9dc7ee80c5..f7abf94319 100644 --- a/poi/src/main/java/org/apache/poi/util/IOUtils.java +++ b/poi/src/main/java/org/apache/poi/util/IOUtils.java @@ -460,7 +460,7 @@ public final class IOUtils { public static long copy(InputStream srcStream, File destFile) throws IOException { File destDirectory = destFile.getParentFile(); if (!(destDirectory.exists() || destDirectory.mkdirs())) { - throw new IllegalStateException("Can't create destination directory: "+destDirectory); + throw new IllegalStateException("Can't create destination directory: " + destDirectory); } try (OutputStream destStream = Files.newOutputStream(destFile.toPath())) { return IOUtils.copy(srcStream, destStream); @@ -581,7 +581,7 @@ public final class IOUtils { throw new RecordFormatException("Can't allocate an array of length < 0, but had " + length + " and " + maxLength); } if (length > (long)Integer.MAX_VALUE) { - throw new RecordFormatException("Can't allocate an array > "+Integer.MAX_VALUE); + throw new RecordFormatException("Can't allocate an array > " + Integer.MAX_VALUE); } checkLength(length, maxLength); } @@ -643,7 +643,7 @@ public final class IOUtils { throw new RecordFormatException(String.format(Locale.ROOT, "Tried to allocate an array of length %,d" + ", but the maximum length for this record type is %,d.%n" + "If the file is not corrupt and not large, please open an issue on bugzilla to request %n" + - "increasing the maximum allowable size for this record type.%n"+ + "increasing the maximum allowable size for this record type.%n" + "You can set a higher override value with IOUtils.setByteArrayMaxOverride()", length, maxLength)); } @@ -651,7 +651,7 @@ public final class IOUtils { throw new RecordFormatException(String.format(Locale.ROOT, "Tried to read data but the maximum length " + "for this record type is %,d.%n" + "If the file is not corrupt and not large, please open an issue on bugzilla to request %n" + - "increasing the maximum allowable size for this record type.%n"+ + "increasing the maximum allowable size for this record type.%n" + "You can set a higher override value with IOUtils.setByteArrayMaxOverride()", maxLength)); } } diff --git a/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java b/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java index b9e7d3050a..1c17bb1506 100644 --- a/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java +++ b/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java @@ -36,6 +36,7 @@ import org.apache.poi.hssf.usermodel.HSSFPatriarch; import org.apache.poi.hssf.usermodel.HSSFSheet; import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.ss.usermodel.Sheet; +import org.apache.poi.util.RecordFormatException; import org.apache.poi.util.StringUtil; import org.apache.tools.ant.util.NullOutputStream; @@ -57,6 +58,7 @@ class TestBiffDrawingToXml extends BaseTestIteratingXLS { excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-5436547081830400.xls", IllegalArgumentException.class); excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-5889658057523200.xls", IndexOutOfBoundsException.class); excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-4977868385681408.xls", IllegalArgumentException.class); + excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-4734163573080064.xls", RecordFormatException.class); return excludes; } diff --git a/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffViewer.java b/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffViewer.java index 77f41647b7..33f28455a8 100644 --- a/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffViewer.java +++ b/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffViewer.java @@ -48,6 +48,7 @@ class TestBiffViewer extends BaseTestIteratingXLS { excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-6137883240824832.xls", IndexOutOfBoundsException.class); excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-6483562584932352.xls", IndexOutOfBoundsException.class); excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-5816431116615680.xls", RecordFormatException.class); + excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-4734163573080064.xls", IndexOutOfBoundsException.class); return excludes; } diff --git a/poi/src/test/java/org/apache/poi/hssf/dev/TestFormulaViewer.java b/poi/src/test/java/org/apache/poi/hssf/dev/TestFormulaViewer.java index 29b31dec81..994a09e8ea 100644 --- a/poi/src/test/java/org/apache/poi/hssf/dev/TestFormulaViewer.java +++ b/poi/src/test/java/org/apache/poi/hssf/dev/TestFormulaViewer.java @@ -45,6 +45,7 @@ class TestFormulaViewer extends BaseTestIteratingXLS { excludes.put("43493.xls", RecordInputStream.LeftoverDataException.class); // HSSFWorkbook cannot open it as well excludes.put("44958_1.xls", RecordInputStream.LeftoverDataException.class); excludes.put("protected_66115.xls", EncryptedDocumentException.class); + excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-4734163573080064.xls", IndexOutOfBoundsException.class); return excludes; } diff --git a/poi/src/test/java/org/apache/poi/hssf/dev/TestRecordLister.java b/poi/src/test/java/org/apache/poi/hssf/dev/TestRecordLister.java index d244f3ff5a..eab59c339f 100644 --- a/poi/src/test/java/org/apache/poi/hssf/dev/TestRecordLister.java +++ b/poi/src/test/java/org/apache/poi/hssf/dev/TestRecordLister.java @@ -51,6 +51,7 @@ class TestRecordLister extends BaseTestIteratingXLS { excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-6137883240824832.xls", RecordFormatException.class); excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-6483562584932352.xls", RecordFormatException.class); excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-5816431116615680.xls", RecordFormatException.class); + excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-4734163573080064.xls", IndexOutOfBoundsException.class); return excludes; } diff --git a/test-data/spreadsheet/clusterfuzz-testcase-minimized-POIHSSFFuzzer-4734163573080064.xls b/test-data/spreadsheet/clusterfuzz-testcase-minimized-POIHSSFFuzzer-4734163573080064.xls new file mode 100644 index 0000000000000000000000000000000000000000..26f9566e238aec480032640b5d5f79b7c51a8b28 GIT binary patch literal 7168 zcmeHL%}Z2K6hCi#@68)Oa+EOBw3m))rTJZvAmbMiMq@f4p&?lV#^@-6LT=KEf}n!7 zwb4zsXw$-IA&fdJgDB_^&=##*NJyo=e&@}+=V-%F8{6D7{N|i{?!D*T`@8qt#rgCx zXKnUP!51RZiTa^bfT)?$XxnhbTc(R7CH~Ux-JPvIUniYrPlCK8jp*k2>dDFKCU3EvMRP+=qMEoA%h~iwqbh2ozQN&% zz%`v*XbtpqcMY7wK1&GgElSsPN{2tm0+P5klK6JJq7;oJ{)+nb-2>3-xZKu#waeep z-qq69*>a)d&!{^T?VUWe*YOu^KATjsg-#xm26AOU0r&=hO{N$;5jY~s05X9r-~f;f z;0jOpbn@98h}PD{|xk5z}ov17MCC`3ASxU;uiB^N=bEzW&V@i zFf^0U)RS=RyH1|dif6P#vskk$EhD9lr0nI+89t|Y+^{v8_u5UPIFoUlMF)kT!!#Ki zx#xf9UN6r^?=8QM-d~<$zVmQnta!YMRI*t2Z2i%l_fqV=m^O#@;`?C=(jfU@1yfCr z^~Ihidt5F#B}EAG2KmMU+Ymp}T6=j^5oO_ns8oSXmF)|VejTz;!qcRZ9pXKCtX-IX|O)C{2oGa6@1XPi7GJgCJ zu(%htjh~ce6j3QclSd}Uv_~Sv>*4)*=&9muRSdi>g_xw;({!&3ZI_|&=T%yeIq~N= z(+oK)9tli4rYjLM7Kw@XwyGS77vQubdA#_VSb$()-xv_rSuh|N5DW+g1OtKr!GK^u zFd!HZ3T#IZQGAZ^n1;sU#=~R1~ z%4Iq)m+2I{xc0*+MVU?^%_`HG-CX9IGOL-FNOjCh&5m4n5`{f)P(gqVK*%ctgNWa?qr5V^uA|bcKN=Mo!N);+xK~%^L^)A zhQUem;H3HZ>((@9kH8;`XzcIa-M7D|*Sqhah!>}%^o?1E!p}qsQ8KbUwyeW!aO^DfTLUbpMoc6ZKgeg}F z?M)2eq{NrSSJ;=edMY|Z&B83E3PS5}5iX|sQ-iJu5g{xYq0g~YT1F1_jXkv`$`mj5 zN`V6f_%1|))C4j2EtuyP%xhq;n4}4sr967lEMGDmn8jO4@hRd>d=IQ7uIQQrD-*6_ zgDW$x_5@eqB2~P~KH5+y#VskNiYMK2VTxM@Qx0IgTT;wfj-hJ^#duUq6vuc}OmuKX zF^>mVlyZSrbk(60uP()_QmWOY%_6*^-QZ@tfz9||h>*)<9L3;KAf?|x%a>XHnMk<~ z&)gC^t#CRmzbjh4Y|`GsIl6D;vWd4L%qolIIViQtV)+iVlZ?Val6T<*$rxNFc@Lsv zWrlqm?WfQlA;LsDzRW5N9|sIO536xRjzcHO`|#*OLzK=yx5y?wfD-}3ILU`_b)mtc zGsGyvN01Yz8fr;ChIW+TmaIAhJrOo>0iF&Rj*?u2(S?RMok3HEPe5v_!A|li)Sv_# z;&p}uWw-& zBtOA;3V!x8%#i#71*s^%f{Wxg7$Er_hEbe<@B>q<9++u${#H>eOzQmSpMVl|ZGZA? zNnjQ(7#>0B{EuO^uBzHl+edJhfP`((*}->v13;>!_^4$#WXnVc&rwNYk0DkU<|`M7#=~p)5>sd z0OH-GV_j?5RioNfEYCJ_z#R3*F}#yuaVA2I;W~z`2%Yf^n{-u8hN|_dDuHL$bHFi5 zO=Nf%!wG^(3^y>0%)+W;-pz1`ppD^XhJLDA#BhtQ zYKx)DrK-|+*2Mu@Hm0UCyoaHaU)~Bp{wdJRCTJV z0-o*UfUI0hUBYk&!wm#)W7x&;kzDC<7BbwatJ-O(+NG+Bcy`x(RUW1;Wq99w6~SVL z-Sbs>^6beavL?)m|Kv1a*x+-nvSe@-$s}kilGZ4n<%t)cJv1qOIp$>lV3F*XrfIwX J=296E_CMM-{A&OJ delta 2521 zcmai$e@v8h9LK-k;|@6Pj)&v$jvvP%$eD*&nk3>8AqF9(iKQVbDXbPl54Kw4kJ?z9 zTeIevH~E^YO>1kevbos0)*AmrWf3s`x zmj221k))`M6hzPNtg}^$Q$j=+L{68Kh~1)vrkrFs5EP<5AVgc05Es2dbT$idCR2!K zsAEv{`#YPYd%STD7mBB8g`}4=x~r>T<+y#8T7^}N2WO9M!Y0PMNrzR*B3U@|X3tZ( zboQU?8tUz{$|J8@q$PI5Di2v=kEY6n($OJ#p%fjmBleTs_J0k$Rcqgc?bgd2&ciP*e=Vl44qx zk&T@x!a~j=IyYb*C5c>mSe&A8|-me<(w87zioF*J*z zSu#w^8FeFW$#s+Eb6DJ(#jRP~nuVS}+R+y(Ld)qM?PklDuy{0!N3(b|OO|Q*N{zW? zx%^srGwHs6qiQ%tRfOYPb%0|`9pm^;O=Z&L_o~E0@q=ph5In2)aQvv=<@hOf-XmYP z$a8A1mk!UX0giDs;iVN9RFRM3XVuIxq4xUd+%IZ?<5xAoaZ!1*DSlHm9G6st!kEO) z7MeS|tPW?({6$lMrEy3{1N7-DrS0-VIP(}TGPn4IEEbp5wE*3}RKOZc zmK6}R1FizxQ9#Sn0PhCePcZBNtc^o@hxq>0ChWA%+^G|@br4Ll)dhGDpnopGbijJR zdV*mCaJ5#oI-#n;tjfS_0|W=z>IS?QaF}5x;C+D8T;%~=qgAa*s9I}QWnp$L1nUaP zItTE6z^52`0oMT@CK&bsHfmLk303ROs%*@zkI(iMku(SF0Z2<3<^nzl*hDa#2e?6N z+K|w+(QNW#b|VBw*_sdd5a1}o0ALef>O86{0NkWiZAz%39|ZJeJQuUg5NsoXQ3$vh z@Og$sfLj3H;i`FnTeYgK2~{m-RWW8;Ajl{tYY^~Zz{+B|)0hvqZKjGQ!?yr#*Q&NB zRJEE_3ozR{Qx&AD5MbL(RZtE_d#z>v^B;VBzN|=c(wAU0xqI@aE_qaANP1LLNIFyZ ey|ARe@7YlqEwpCEdP1^WT5k2nu9nE;q<;Y&>E4h4