From a07ed9e86474da98f204efadfd5b9327009a0d21 Mon Sep 17 00:00:00 2001
From: Tim Allison <tallison@apache.org>
Date: Tue, 25 Jul 2017 20:26:57 +0000
Subject: [PATCH] 61338 -- avoid infinite loop triggered by fuzzed wmf file

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1802997 13f79535-47bb-0310-9956-ffa450edef68
---
 .../poi/hwmf/usermodel/HwmfPicture.java       |  21 +++++++++++++-----
 .../org/apache/poi/hwmf/TestHwmfParsing.java  |  18 ++++++++++++---
 test-data/slideshow/61338.wmf                 | Bin 0 -> 9090 bytes
 3 files changed, 31 insertions(+), 8 deletions(-)
 create mode 100644 test-data/slideshow/61338.wmf

diff --git a/src/scratchpad/src/org/apache/poi/hwmf/usermodel/HwmfPicture.java b/src/scratchpad/src/org/apache/poi/hwmf/usermodel/HwmfPicture.java
index 78aca4241..77ba355e1 100644
--- a/src/scratchpad/src/org/apache/poi/hwmf/usermodel/HwmfPicture.java
+++ b/src/scratchpad/src/org/apache/poi/hwmf/usermodel/HwmfPicture.java
@@ -35,9 +35,11 @@ import org.apache.poi.hwmf.record.HwmfRecord;
 import org.apache.poi.hwmf.record.HwmfRecordType;
 import org.apache.poi.hwmf.record.HwmfWindowing.WmfSetWindowExt;
 import org.apache.poi.hwmf.record.HwmfWindowing.WmfSetWindowOrg;
+import org.apache.poi.util.IOUtils;
 import org.apache.poi.util.LittleEndianInputStream;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
+import org.apache.poi.util.RecordFormatException;
 import org.apache.poi.util.Units;
 
 public class HwmfPicture {
@@ -59,7 +61,13 @@ public class HwmfPicture {
                 break;
             }
             // recordSize in DWORDs
-            long recordSize = leis.readUInt()*2;
+            long recordSizeLong = leis.readUInt()*2;
+            if (recordSizeLong > Integer.MAX_VALUE) {
+                throw new RecordFormatException("record size can't be > "+Integer.MAX_VALUE);
+            } else if (recordSizeLong < 0L) {
+                throw new RecordFormatException("record size can't be < 0");
+            }
+            int recordSize = (int)recordSizeLong;
             int recordFunction = leis.readShort();
             // 4 bytes (recordSize) + 2 bytes (recordFunction)
             int consumedSize = 6;
@@ -82,10 +90,13 @@ public class HwmfPicture {
             
             consumedSize += wr.init(leis, recordSize, recordFunction);
             int remainingSize = (int)(recordSize - consumedSize);
-            assert(remainingSize >= 0);
-            if (remainingSize > 0) {
-            	// skip size in loops, because not always all bytes are skipped in one call 
-                for (int i=remainingSize; i>0; i-=leis.skip(i));
+            if (remainingSize < 0) {
+                throw new RecordFormatException("read too many bytes. record size: "+recordSize + "; comsumed size: "+consumedSize);
+            } else if(remainingSize > 0) {
+                long skipped = IOUtils.skipFully(leis, remainingSize);
+                if (skipped != (long)remainingSize) {
+                    throw new RecordFormatException("Tried to skip "+remainingSize + " but skipped: "+skipped);
+                }
             }
         }
     }
diff --git a/src/scratchpad/testcases/org/apache/poi/hwmf/TestHwmfParsing.java b/src/scratchpad/testcases/org/apache/poi/hwmf/TestHwmfParsing.java
index 8aac3b29b..7e3bf2ec6 100644
--- a/src/scratchpad/testcases/org/apache/poi/hwmf/TestHwmfParsing.java
+++ b/src/scratchpad/testcases/org/apache/poi/hwmf/TestHwmfParsing.java
@@ -20,6 +20,7 @@ package org.apache.poi.hwmf;
 import static org.apache.poi.POITestCase.assertContains;
 import static org.junit.Assert.assertEquals;
 
+import javax.imageio.ImageIO;
 import java.awt.Dimension;
 import java.awt.Graphics2D;
 import java.awt.RenderingHints;
@@ -38,8 +39,6 @@ import java.util.Locale;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipInputStream;
 
-import javax.imageio.ImageIO;
-
 import org.apache.poi.POIDataSamples;
 import org.apache.poi.hwmf.record.HwmfFill.HwmfImageRecord;
 import org.apache.poi.hwmf.record.HwmfFont;
@@ -52,6 +51,7 @@ import org.apache.poi.sl.usermodel.PictureData.PictureType;
 import org.apache.poi.sl.usermodel.SlideShow;
 import org.apache.poi.sl.usermodel.SlideShowFactory;
 import org.apache.poi.util.LocaleUtil;
+import org.apache.poi.util.RecordFormatException;
 import org.apache.poi.util.Units;
 import org.junit.Ignore;
 import org.junit.Test;
@@ -66,7 +66,19 @@ public class TestHwmfParsing {
         List<HwmfRecord> records = wmf.getRecords();
         assertEquals(581, records.size());
     }
-    
+
+    @Test(expected = RecordFormatException.class)
+    public void testInfiniteLoop() throws Exception {
+        File f = POIDataSamples.getSlideShowInstance().getFile("61338.wmf");
+        FileInputStream fis = null;
+        try {
+            fis = new FileInputStream(f);
+            HwmfPicture wmf = new HwmfPicture(fis);
+        } finally {
+            fis.close();
+        }
+    }
+
     @Test
     @Ignore("This is work-in-progress and not a real unit test ...")
     public void paint() throws IOException {
diff --git a/test-data/slideshow/61338.wmf b/test-data/slideshow/61338.wmf
new file mode 100644
index 0000000000000000000000000000000000000000..8873072a0934cd4ec9ee74c17438b2cdae412361
GIT binary patch
literal 9090
zcmd5?2~<>Pvi`eqCB|(unm3+FyXBc#jFT9TK9jg688dE4Bp7v68WC|Nif`0tV2nmj
zjtXvoii&8&4K>k3gBxz+8kZ;r6jVe6QDg}_O@HsJd%4&&jWL;-b6zEN|GmEYtLm@+
z-bRzEuaw)-CQ8*&_ZlgZJYTHSNF95@bHLkuOwy%HYC$i(R4VcXtxGFSos^oWH#tYC
zG34$QZ7CWyRH_>REN=~XTcpU^SJ_`zsg@4YRL?p}&2v+#vstM(b;!!$q=<Uz^@Lj5
zf(8}6&$E;|05z3mWMie;vXrI0$riHFmb$PCf`jCs9@3@L<GRQbl09`MN0Jp~TUZEp
zs3@gwVh>f;#-*rAl%xM&wcB;89=pXJmVYCQJdyiE8a8ZL=&Dt#5FQ?mu&^+MhK6dq
za^*^_Sg``jmoLY%Wy>^Px^yW*LP9hS4i3hWB}+73ym&E$7cE*;$?w1a{$2uC@v$pK
z)~s1WlmGikbRnL_6{cffL_~xJURqj;l9Cb>k&7L;u(%j5xv&rgHLwVDjc=;W$ZyWb
zZKu>TZRno=TDo*4Y_@Wg*=!oV=-`N7OnifjSg*!KMJROQ8t_fZy=g1uwP=|NI&fHx
zoVP#?n=xzX*5kNaWJi8ZCa#~3!Py<3<5<++u{U@Gz6lwP!%?$vZf_{=UjJUxqZ+>G
z<N{yfp{o74_xW44sf8y`saj26;_0a+eu!AT5(#nZaVX3i7o)96+B+2YV&BEX!#$C)
zzb~$DAAqxK24Y{Z2X&c^%+%}EZ6H2!&_VX^j9=&XuXR(gX=$o|;O9!YTL^9XZPQwR
zhzcBt_?;t>b*euKu8u(QO+S>TOhH-7Wby=*-58Cc%R`WTq$e(}eFM7|k3jOJL)CR4
zzF`N!m$N|(!O^I(r|)=mkobM32h#s9wrX1Y<;m|f-I}<w7qY+iMBy!el%)s4mN5?{
z_vWD}WiASDenNfz4&3<|wj>`Eoav9`=-2S=7XxuS{-kSvP!Cu5&4&9;P;tcnTlm@r
zbsKi~?b~$ImFU-h%DONN1-AlF_TY1rJqkhUnLwP4o`8d)UO2qUiu2nhAoWxLN|OU%
zOZG$Y51vT<x);7)ItE!8DK2AE@FhQR;}(7QN;Ut>2SaL|BL0%A@qIeASh8<<-`fw)
zcq0GSG?YGE02{Hd?GM2Do%8V^HJ)do7zKG*xO4Rgj;x!6ANP(&;k79!7ds_SK+-qE
zaVT;o{ZOQx7st31e92Re`0lM<ur$1wmS)<LmeG6ClppUL9)z64i72`M8EiQrNWb(M
z&hH3BX-N@E%gS_SPLRBio0W#|R(fI27w=);vXS_PXCrpiM4Z^R0C#WjT*#TNIH#hA
zEBsodS9iz7)jpY?-DqukampEQ<fqO;S#B^n43~C(j0Y)KG(5r7@{!~uVaj)8SNXwt
z|B|a6_irY(A2rVJxf>4)YVR(={39{QzaI>H@miGUM&RW7@x&^rws#HR(8EEODmqX<
zXMDLc9PuR%>NjfAWbl<cxo#Ir;I_qv3rXu>D_jqhZXmudPOck^;}JeMw#ElX*Nj2j
zDsLPKx8iV^6^9s)5|an$hu9UP@ZIuJ*iT=a-Sve_`@6yy8w4!>^kS!bB~ZuhX#Z^{
za<kXKZi@u$QLfp>{xXgitpoB_0uOjTlRrkz>5q`SsXGpa`##Rs6)_<GmwVHB|6+px
z&ljWS-OsT<n{J2t1_(HFj&Z&Y_EKV$2-6;wOw5wC#9j;gPvHRL6c4{ZdCD~8pZf?&
zo8G{Y2*$AEoly~ga(7q3e^Jnt8r`e;=Lt7=6?qnvL*3%b&Ng7>h5b0bZURn4`q2;L
z=m%e%SX<!>`r|n3$JUNRamsukYd&*<KXOlc;8OToIJ^C`s`%7JNZua^XAw+Cdh?}c
zh928GHtA~qHS{kfega_HY{$~|D|m)Vw0ltQ!ZMyggNus`H64nQ=hFWFK*@DKWF6{{
z<I4si;rQ0dGh)P~;LCf=?_A(FZ|L^IV&C8Ix93ISKw&!iP+fHysI|$4@4ml6OkxYZ
zgARAE9zgt8%sZ(;z`dEszc>;ZU-iO~6~pl;<7Oqk;Rges`?fgl-KcwXZ8Y=LmJzq?
zg*&mKFb{7OQit!U!^Se~ITnZ9%oI(JysW!OxwwZu@k8#_Pk@X$D7`+BchMl63m=Hf
z-!65TCj?*K<!Zuj>~3x`x!-Rh(l1QQE-U^TVFd+vU39nz)ZT2vnus7A3irXe%@dKZ
zF91JX59Gc-W&WRvywk&Qb6Y>0*f0q<($o14R7C$*%~ygi?~^s*iyhjwsPjs|z~-Cs
z6KA80c35AKi*BMrJngW%9NSJs;qDdQgLme`exGrdHUoKQN8t9(zcMy`k#_qE!ct3-
zm6gZyr{hb`EG7%dJMEM34IRE(<bkx~Ghu(U76%Ga&^4br++s}bv12pu`8!vGk#p@6
zWS{y7KSX)rbkr0)Ou4MteO$5)4<6)d_>Or}@K>*1{j>O@LrnBK+&mhNt2_L0=i~zX
zn0^3*v&&HbF7rdI9X>I!h>f0w1ED^+uqOohkM3!)c|S7;1AZt&dio<|J<3M%&6~Iy
zAFq`K^ryU6Q~b}qfA83_L)1m#qdQ2uxEt3GF2!Z)6PT2SMoGn}eX1A@cNZf3NFvUi
zyFpIEg$v2p`9lVtKU9v)%v>BibO;+ZY|!>Bd1vq5y^!~6;{WXTM@&qNhA)ru^72Zy
zv3|M0hLJhI^S6PPjLqMjDaVk@r5JRr3@y$99`QEh<`!VvHYPEWoWJbsZ0+tJK79Bp
zvG0>;7YD20+wGP3j=Cfjmtz^@V)#R#W11b^nY)9tY$z-&)^e0#L&2B0n>~AW{F87U
z#~ktPf?tCUhCX5!8*`JvnVFd|WNh5H5p(CxeQIpc!&s{s@K-#Nl9CX2^eDD(-;RX~
z7ycq_!T0g;38fBKMUR-6I@(^up28wiSp2(n>sF1oY}tb7=xB{MZ{CbAzx)!LHf_SF
zQKLdd-v39ELs;><g}G@(-Nf>JvOg<QFQDyNR8oj^3iVWNhA1yJPEAphSpGUnuqHIP
z=Kb-^*Io<X*;3tYUCSF`3D(oB%STuw%XZNT)`u)JI3|5E=RDhS8xyR8d7E`1>G%Is
z`hHTEcD2-g|BzBhTgQ0?bM$eMXV4DL2|ki&m2JsyCbDKT+m?<uyj!xq(>>AJGcVp-
z#yh3V`Ou$z?Z|&)zofCjtQW;5TD!8G-6_#ppJ4o1Mh#`VbC*Qx5Y}am$gTHY&wA31
zWIZ5v{D))ySI<N3*(ZJw48!(j)@!hRP+i)7fcijLl!xla=R5R406pQSyt&FH@%!GH
zu3J@5j9JC;=co5F>KbK-X^(AV=0+;wLk(cP9mnK{xTNN$SQ+cY_9F7Wa@sXAPXp-0
z=d<MV>~F30Jbr|*?|fN;HD}{O>XWCW{i!X%I*;WJa=n0-Y96P{x`kcy%|XUe%8ul?
zr(2@68U3Pn>+)rO9LL(1BxrsT8A9{^Y4(m~d6#2<ElRNZH<=?Q)hM5R&bF69!}byf
zHQ0Wk*nY4YKwD2%zN$Y<4_)Xle9j%7V11SR7l(Cz!}NAv?osTT>6u_1I=8cuIg_UA
zV{4=IQhZ_9K4?X*wl}eTQ?jG&O>7Grw(s1lF1MF!ev<7&I46i}ghckQvHhn6HEe&W
z?Hse{AmqySVsFFta#m}wz2ux0|9Z|*%Fdi72CG9BY$5&>tezpunAD9qqL0iIHpsCp
zb=jN9ky5TI7FEcdt@Dh_$W;DdTrGCqoGz0ETW~c;R+J)J`i-1BnsuS&gL*evM0p>U
zCt5E*XL38u`U$dNY#}$YF*l9QQ_f?TIOZJpvcL9ZBmexCbslYv+VJ@t*%*`DCFe~@
z#w6Z4l6Buc+uOtA+1_kwW3re2WRY*sSXN}hXmWt^QPcQ*R}EHgF@ud_pJ8iBR#KPJ
zur+^|*Xo8oY*xmM%;RfN6}wm1EJ*cK|NRSdrHHOZ{$aeIXm8o;rPVD}<5XWy*`tu*
zb17Sfwp7<5yG?h0uJy|@CU%m&xLPA~nw_rdcUj{|eJM|=M60tr|0}+=9d)qCBI>yH
ziaytIKL2tty~C0&>I7wZ1$CQlV>u(Fi+WwwRFL@ik6bVFs-Cef)nZ2M?HngY`_CL}
zOE&x@_L8_6O*Y1)?Ho78n4F`U;zrg@9vkMn%O}w)`WkFtv8ysBByMVmiG>mq|9TE@
z{j#Amp?Zpc+b||Xf1#RU!dPR(g!39BCJZ^cw5m_Xm~fOQG2tlBBFmg&!k8~Z&N@ph
z#%b}U`$~&%<`L0pKl}HR50Yh$a~=_UcOZ*TBt8tE82%UE$xYdoJepjS|7Ctn{vXV-
zz;GSIkk774p9}7P_U@?e^A7a6=q&WKK6hSY_}q|V*2PtMKFQ~f@+`9IXMFBB-`VHl
zzc~M`_4?M)?<L9ZTUxa?U2@@bvA5xK@xOBpFxp-mlRRknO!g`9AZ(0{Vcj_<j4_%2
zq?qW)_i#B+hJ045+-Zi6&Us7bi4LR5e;`Z3?ndrT7EHd$YImA3CTFrHJw(oU<<ECQ
z{awJgx7xEUHsw2U(Az96WO+yHr@UX9!gt6)<cVq~dz6xKxl`MbtKW}$<_iq(ScxO^
zXOJh7`>6j>BY2li<+p$@pI-bbvrwWRN9Crq&{H#1SJg$m#CiRQDhN|q`mko<9Xx~D
zPvq*UTK@>H^r_O<QEU4UOK{~KL2RDIpm*9AWmMf)L(XeP7Ls*Rm$h!DdRne{MhkfH
zP6KlvHCF4ZU)g6e6XJ27Syq}&Ce?~%nYp0i)~~qld-AK)YuuDcUL34}Wc{B^%GLUy
zx?Dft1B*vL%R5s7#`$@>5}~hmiLZYh4Dg%m+uyIo&jNkD*%iM2hWk|dyJ;4beTdzf
zndN+m<c_qA{2PQ<+@+_!)~9IG-YURnTEM%5-+E^h`|5C2Epi?HSC_1>6q~$Do8~*(
zRDXkdDw~KzLl?c)mUq6Pm3aLXcj?jbbgfqYBkb~$S#m-Qeyof5w~8+M&HYsNmHjD|
z!$0O4_=ZLv+V`nw;=xlcqq-(N(4b<j%h>n@zR@vr_=jZ!^Usx*@v78EzLLdDURAW{
z2t#AtpW<O-C%QDzS5DC@?k_WI>2aX%V7$NY1UJp6`q^kK|C)t#P&-Q|v$@R&Gp7ak
dOj5t;(W3|Z`Rq|ouWI!k8q%L=Gs$#H{U5E)RDb{g

literal 0
HcmV?d00001