bug 422118 crash reading malformed zip archives. r=biesi, sr=bzbarsky, a=schrep
authordveditz@cruzio.com
Wed, 19 Mar 2008 12:28:38 -0700
changeset 13296 31fc77a3eccc3ca595888d0313c538a79dc7a6a0
parent 13295 d0588f88b33d03dfdcf2acc4140b08e8b34ed7bf
child 13297 596144d873cd4bedb7f57c5c314063115ee75fd2
push idunknown
push userunknown
push dateunknown
reviewersbiesi, bzbarsky, schrep
bugs422118
milestone1.9b5pre
bug 422118 crash reading malformed zip archives. r=biesi, sr=bzbarsky, a=schrep
modules/libjar/nsJARInputStream.cpp
modules/libjar/nsZipArchive.cpp
modules/libjar/zipstruct.h
--- a/modules/libjar/nsJARInputStream.cpp
+++ b/modules/libjar/nsJARInputStream.cpp
@@ -214,25 +214,31 @@ nsJARInputStream::Read(char* aBuffer, PR
     if (mDirectory) {
         rv = ReadDirectory(aBuffer, aCount, aBytesRead);
     } else {
         if (mInflate) {
             rv = ContinueInflate(aBuffer, aCount, aBytesRead);
         } else {
             PRInt32 bytesRead = 0;
             aCount = PR_MIN(aCount, mInSize - mCurPos);
-            if (aCount) {        
+            if (aCount) {
                 bytesRead = PR_Read(mFd, aBuffer, aCount);
                 if (bytesRead < 0)
                     return NS_ERROR_FILE_CORRUPTED;
                 mCurPos += bytesRead;
+                if (bytesRead != aCount) {
+                    // file is truncated or was lying about size, we're done
+                    PR_Close(mFd);
+                    mFd = nsnull;
+                    return NS_ERROR_FILE_CORRUPTED;
+                }
             }
             *aBytesRead = bytesRead;
         }
-        
+
         // be aggressive about closing!
         // note that sometimes, we will close mFd before we've finished
         // deflating - this is because zlib buffers the input
         // So, don't free the ReadBuf/InflateStruct yet.
         if (mCurPos >= mInSize && mFd) {
             PR_Close(mFd);
             mFd = nsnull;
         }
--- a/modules/libjar/nsZipArchive.cpp
+++ b/modules/libjar/nsZipArchive.cpp
@@ -901,34 +901,38 @@ nsresult nsZipArchive::BuildFileList()
     // read the fixed-size data
     //-------------------------------------------------------
     ZipCentral* central = (ZipCentral*)(buf+pos);
 
     PRUint16 namelen = xtoint(central->filename_len);
     PRUint16 extralen = xtoint(central->extrafield_len);
     PRUint16 commentlen = xtoint(central->commentfield_len);
 
+    //-- sanity check variable sizes and refuse to deal with
+    //-- anything too big: it's likely a corrupt archive
+    if (namelen > BR_BUF_SIZE || extralen > BR_BUF_SIZE || commentlen > 2*BR_BUF_SIZE)
+      return ZIP_ERR_CORRUPT;
+
     nsZipItem* item = CreateZipItem(namelen);
     if (!item)
       return ZIP_ERR_MEMORY;
 
     item->headerOffset  = xtolong(central->localhdr_offset);
     item->dataOffset    = 0;
     item->size          = xtolong(central->size);
     item->realsize      = xtolong(central->orglen);
     item->crc32         = xtolong(central->crc32);
     item->time          = xtoint(central->time);
     item->date          = xtoint(central->date);
     item->isSynthetic   = PR_FALSE;
     item->hasDataOffset = PR_FALSE;
-    item->compression   = (PRUint8)xtoint(central->method);
-#if defined(DEBUG)
-    /* Make sure our space optimization is non lossy. */
-    PR_ASSERT(xtoint(central->method) == (PRUint16)item->compression);
-#endif
+
+    PRUint16 compression = xtoint(central->method);
+    item->compression   = (compression < UNSUPPORTED) ? (PRUint8)compression
+                                                      : UNSUPPORTED;
 
     item->mode = ExtractMode(central->external_attributes);
 #if defined(XP_UNIX) || defined(XP_BEOS)
     // Check if item is a symlink
     item->isSymlink = IsSymlink(central->external_attributes);
 #endif
 
     pos += ZIPCENTRAL_SIZE;
@@ -939,16 +943,21 @@ nsresult nsZipArchive::BuildFileList()
     //-------------------------------------------------------
     PRInt32 leftover = byteCount - pos;
     if (leftover < (namelen + extralen + commentlen + ZIPCENTRAL_SIZE)) {
       //-- not enough data left to process at top of loop.
       //-- move leftover and read more
       memcpy(buf, buf+pos, leftover);
       byteCount = leftover + PR_Read(mFd, buf+leftover, sizeof(buf)-leftover);
       pos = 0;
+
+      if (byteCount < (namelen + extralen + commentlen + sizeof(sig))) {
+        // truncated file
+        return ZIP_ERR_CORRUPT;
+      }
     }
 
     //-------------------------------------------------------
     // get the item name
     //-------------------------------------------------------
     memcpy(item->name, buf+pos, namelen);
     item->name[namelen] = 0;
 
--- a/modules/libjar/zipstruct.h
+++ b/modules/libjar/zipstruct.h
@@ -127,11 +127,12 @@ typedef struct ZipEnd_
 #define SHRUNK            1
 #define REDUCED1          2
 #define REDUCED2          3
 #define REDUCED3          4
 #define REDUCED4          5
 #define IMPLODED          6
 #define TOKENIZED         7
 #define DEFLATED          8
+#define UNSUPPORTED       0xFF
 
 
 #endif /* _zipstruct_h */