Bug 1166900 - Better string length check in nsZipArchive::GetDataOffset. r+a=dveditz
authorAndrea Marchesini <amarchesini@mozilla.com>
Thu, 04 Jun 2015 15:04:10 +0100
changeset 201220 ec83c431fcec693358b7ac54993e015286c49965
parent 201219 f857e7a87750ce79981571f2d191586e9705cf27
child 201221 e52482d1102d51f6bea36848dc354afdb2ab9398
push id282
push userryanvm@gmail.com
push dateFri, 05 Jun 2015 15:09:44 +0000
treeherdermozilla-esr31@ec83c431fcec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1166900
milestone31.7.0
Bug 1166900 - Better string length check in nsZipArchive::GetDataOffset. r+a=dveditz
dom/file/ArchiveZipFile.cpp
modules/libjar/nsZipArchive.cpp
--- a/dom/file/ArchiveZipFile.cpp
+++ b/dom/file/ArchiveZipFile.cpp
@@ -97,17 +97,18 @@ ArchiveInputStream::Init()
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   mData.sizeToBeRead = ArchiveZipItem::StrToInt32(mCentral.size);
 
   uint32_t offset = ArchiveZipItem::StrToInt32(mCentral.localhdr_offset);
 
   // The file is corrupt
-  if (offset + ZIPLOCAL_SIZE > mData.parentSize) {
+  if (mData.parentSize < ZIPLOCAL_SIZE ||
+      offset > mData.parentSize - ZIPLOCAL_SIZE) {
     return NS_ERROR_UNEXPECTED;
   }
 
   // From the input stream to a seekable stream
   nsCOMPtr<nsISeekableStream> seekableStream;
   seekableStream = do_QueryInterface(mData.inputStream);
   if (!seekableStream) {
     return NS_ERROR_UNEXPECTED;
@@ -132,17 +133,18 @@ ArchiveInputStream::Init()
   memcpy(&local, buffer, ZIPLOCAL_SIZE);
 
   // Seek to the real data:
   offset += ZIPLOCAL_SIZE +
             ArchiveZipItem::StrToInt16(local.filename_len) +
             ArchiveZipItem::StrToInt16(local.extrafield_len);
 
   // The file is corrupt if there is not enough data
-  if (offset + mData.sizeToBeRead > mData.parentSize) {
+  if (mData.parentSize < mData.sizeToBeRead ||
+      offset > mData.parentSize - mData.sizeToBeRead) {
     return NS_ERROR_UNEXPECTED;
   }
 
   // Data starts here:
   seekableStream->Seek(nsISeekableStream::NS_SEEK_SET, offset);
 
   // The file is compressed or not?
   mData.compressed = (ArchiveZipItem::StrToInt16(mCentral.method) != 0);
--- a/modules/libjar/nsZipArchive.cpp
+++ b/modules/libjar/nsZipArchive.cpp
@@ -632,28 +632,30 @@ MOZ_WIN_MEM_TRY_BEGIN
       return NS_ERROR_FILE_CORRUPTED;
 
     // Read the fixed-size data.
     ZipCentral* central = (ZipCentral*)buf;
 
     uint16_t namelen = xtoint(central->filename_len);
     uint16_t extralen = xtoint(central->extrafield_len);
     uint16_t commentlen = xtoint(central->commentfield_len);
-
-    // Point to the next item at the top of loop
-    buf += ZIPCENTRAL_SIZE + namelen + extralen + commentlen;
+    uint32_t diff = ZIPCENTRAL_SIZE + namelen + extralen + commentlen;
 
     // Sanity check variable sizes and refuse to deal with
     // anything too big: it's likely a corrupt archive.
     if (namelen < 1 ||
         namelen > kMaxNameLength ||
-        buf >= endp) {
+        buf >= buf + diff || // No overflow
+        buf >= endp - diff) {
       return NS_ERROR_FILE_CORRUPTED;
     }
 
+    // Point to the next item at the top of loop
+    buf += diff;
+
     nsZipItem* item = CreateZipItem();
     if (!item)
       return NS_ERROR_OUT_OF_MEMORY;
 
     item->central = central;
     item->nameLength = namelen;
     item->isSynthetic = false;
 
@@ -774,33 +776,34 @@ const uint8_t* nsZipArchive::GetData(nsZ
 {
   PR_ASSERT (aItem);
 MOZ_WIN_MEM_TRY_BEGIN
   //-- read local header to get variable length values and calculate
   //-- the real data offset
   uint32_t len = mFd->mLen;
   const uint8_t* data = mFd->mFileData;
   uint32_t offset = aItem->LocalOffset();
-  if (offset + ZIPLOCAL_SIZE > len)
+  if (len < ZIPLOCAL_SIZE || offset > len - ZIPLOCAL_SIZE)
     return nullptr;
 
   // -- check signature before using the structure, in case the zip file is corrupt
   ZipLocal* Local = (ZipLocal*)(data + offset);
   if ((xtolong(Local->signature) != LOCALSIG))
     return nullptr;
 
   //-- NOTE: extralen is different in central header and local header
   //--       for archives created using the Unix "zip" utility. To set
   //--       the offset accurately we need the _local_ extralen.
   offset += ZIPLOCAL_SIZE +
             xtoint(Local->filename_len) +
             xtoint(Local->extrafield_len);
 
   // -- check if there is enough source data in the file
-  if (offset + aItem->Size() > len)
+  if (len < aItem->Size() ||
+      offset > len - aItem->Size())
     return nullptr;
 
   return data + offset;
 MOZ_WIN_MEM_TRY_CATCH(return nullptr)
 }
 
 // nsZipArchive::GetComment
 bool nsZipArchive::GetComment(nsACString &aComment)