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 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 000000000..8873072a0 Binary files /dev/null and b/test-data/slideshow/61338.wmf differ