Bug 58156: Possible data corruption in hasPOIFSHeader and hasOOXMLHeader

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1691948 13f79535-47bb-0310-9956-ffa450edef68

5 files changed, 74 insertions, 10 deletions

diff --git a/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java b/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java
index c27ba3cd14..9817c3d818 100644
--- a/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java
+++ b/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java
@@ -354,7 +354,11 @@ public class NPOIFSFileSystem extends BlockStore
      *  has a POIFS (OLE2) header at the start of it.
      * If your InputStream does not support mark / reset,
      *  then wrap it in a PushBackInputStream, then be
-     *  sure to always use that, and not the original!
+     *  sure to always use that and not the original!
+     *  
+     *  After the method call, the InputStream is at the
+     *  same position as of the time of entering the method.
+     *  
      * @param inp An InputStream which supports either mark/reset, or is a PushbackInputStream
      */
     public static boolean hasPOIFSHeader(InputStream inp) throws IOException {
@@ -362,13 +366,13 @@ public class NPOIFSFileSystem extends BlockStore
         inp.mark(8);
 
         byte[] header = new byte[8];
-        IOUtils.readFully(inp, header);
+        int bytesRead = IOUtils.readFully(inp, header);
         LongField signature = new LongField(HeaderBlockConstants._signature_offset, header);
 
         // Wind back those 8 bytes
         if(inp instanceof PushbackInputStream) {
             PushbackInputStream pin = (PushbackInputStream)inp;
-            pin.unread(header);
+            pin.unread(header, 0, bytesRead);
         } else {
             inp.reset();
         }
diff --git a/src/java/org/apache/poi/util/IOUtils.java b/src/java/org/apache/poi/util/IOUtils.java
index f1d5a2378d..b5f22a1a1a 100644
--- a/src/java/org/apache/poi/util/IOUtils.java
+++ b/src/java/org/apache/poi/util/IOUtils.java
@@ -57,7 +57,7 @@ public final class IOUtils {
         // Wind back those 8 bytes
         if(stream instanceof PushbackInputStream) {
             PushbackInputStream pin = (PushbackInputStream)stream;
-            pin.unread(header);
+            pin.unread(header, 0, read);
         } else {
             stream.reset();
         }
diff --git a/src/ooxml/java/org/apache/poi/POIXMLDocument.java b/src/ooxml/java/org/apache/poi/POIXMLDocument.java
index 0352d5c874..b0b5d4454f 100644
--- a/src/ooxml/java/org/apache/poi/POIXMLDocument.java
+++ b/src/ooxml/java/org/apache/poi/POIXMLDocument.java
@@ -127,18 +127,19 @@ public abstract class POIXMLDocument extends POIXMLDocumentPart implements Close
         inp.mark(4);
 
         byte[] header = new byte[4];
-        IOUtils.readFully(inp, header);
+        int bytesRead = IOUtils.readFully(inp, header);
 
         // Wind back those 4 bytes
         if(inp instanceof PushbackInputStream) {
             PushbackInputStream pin = (PushbackInputStream)inp;
-            pin.unread(header);
+            pin.unread(header, 0, bytesRead);
         } else {
             inp.reset();
         }
 
         // Did it match the ooxml zip signature?
         return (
+                bytesRead == 4 &&
                 header[0] == POIFSConstants.OOXML_FILE_HEADER[0] &&
                 header[1] == POIFSConstants.OOXML_FILE_HEADER[1] &&
                 header[2] == POIFSConstants.OOXML_FILE_HEADER[2] &&
diff --git a/src/ooxml/testcases/org/apache/poi/TestDetectAsOOXML.java b/src/ooxml/testcases/org/apache/poi/TestDetectAsOOXML.java
index 4ea2213418..ff4f5dccd8 100644
--- a/src/ooxml/testcases/org/apache/poi/TestDetectAsOOXML.java
+++ b/src/ooxml/testcases/org/apache/poi/TestDetectAsOOXML.java
@@ -19,8 +19,10 @@
 
 package org.apache.poi;
 
+import java.io.ByteArrayInputStream;
 import java.io.InputStream;
 import java.io.PushbackInputStream;
+import java.util.Arrays;
 
 import junit.framework.TestCase;
 
@@ -62,4 +64,22 @@ public class TestDetectAsOOXML extends TestCase
 		assertFalse(POIXMLDocument.hasOOXMLHeader(in));
 		in.close();
 	}
+    
+    public void testFileCorruption() throws Exception {
+	    
+	    // create test InputStream
+	    byte[] testData = { (byte)1, (byte)2, (byte)3 };
+        ByteArrayInputStream testInput = new ByteArrayInputStream(testData);
+        
+        // detect header
+        InputStream in = new PushbackInputStream(testInput, 10);
+        assertFalse(POIXMLDocument.hasOOXMLHeader(in));
+        
+        // check if InputStream is still intact
+        byte[] test = new byte[3];
+        in.read(test);
+        assertTrue(Arrays.equals(testData, test));
+        assertEquals(-1, in.read());
+	}
+
 }
diff --git a/src/testcases/org/apache/poi/poifs/filesystem/TestOffice2007XMLException.java b/src/testcases/org/apache/poi/poifs/filesystem/TestOffice2007XMLException.java
index d475b7aad0..d5d92bd6bb 100644
--- a/src/testcases/org/apache/poi/poifs/filesystem/TestOffice2007XMLException.java
+++ b/src/testcases/org/apache/poi/poifs/filesystem/TestOffice2007XMLException.java
@@ -17,12 +17,16 @@
 
 package org.apache.poi.poifs.filesystem;
 
-import junit.framework.TestCase;
-
-import java.io.*;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PushbackInputStream;
+import java.util.Arrays;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
 
+import junit.framework.TestCase;
+
 /**
  * Class to test that POIFS complains when given an Office 2007 XML document
  *
@@ -38,7 +42,7 @@ public class TestOffice2007XMLException extends TestCase {
 		InputStream in = openSampleStream("sample.xlsx");
 
 		try {
-			new POIFSFileSystem(in);
+			new POIFSFileSystem(in).close();
 			fail("expected exception was not thrown");
 		} catch(OfficeXmlFileException e) {
 			// expected during successful test
@@ -72,4 +76,39 @@ public class TestOffice2007XMLException extends TestCase {
 		    in.close();
 		}
 	}
+    
+    public void testFileCorruption() throws Exception {
+        
+        // create test InputStream
+        byte[] testData = { (byte)1, (byte)2, (byte)3 };
+        InputStream testInput = new ByteArrayInputStream(testData);
+        
+        // detect header
+        InputStream in = new PushbackInputStream(testInput, 10);
+        assertFalse(POIFSFileSystem.hasPOIFSHeader(in));
+        
+        // check if InputStream is still intact
+        byte[] test = new byte[3];
+        in.read(test);
+        assertTrue(Arrays.equals(testData, test));
+        assertEquals(-1, in.read());
+    }
+
+
+    public void testFileCorruptionOPOIFS() throws Exception {
+        
+        // create test InputStream
+        byte[] testData = { (byte)1, (byte)2, (byte)3 };
+        InputStream testInput = new ByteArrayInputStream(testData);
+        
+        // detect header
+        InputStream in = new PushbackInputStream(testInput, 10);
+        assertFalse(OPOIFSFileSystem.hasPOIFSHeader(in));
+
+        // check if InputStream is still intact
+        byte[] test = new byte[3];
+        in.read(test);
+        assertTrue(Arrays.equals(testData, test));
+        assertEquals(-1, in.read());
+    }
 }