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

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 78aca4241bd13ef62e7c3337b07dec02543a2609..77ba355e1cb3ed5dd667d2f30e40ddfdf492355d 100644 (file)
--- 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 8aac3b29b4b016e1652cd96111a5ef257487096a..7e3bf2ec6d7f71b88aef6e77518ea5b6563f173c 100644 (file)
--- 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 (file)
index 0000000..8873072
Binary files /dev/null and b/test-data/slideshow/61338.wmf differ