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 266188 634d32969bd6
parent 266187 f16daa2effd1
child 266189 62bb5056f458
push id4780
push userryanvm@gmail.com
push date2015-06-04 17:55 +0000
treeherdermozilla-beta@62bb5056f458 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1166900
milestone39.0
Bug 1166900 - Better string length check in nsZipArchive::GetDataOffset. r+a=dveditz
dom/archivereader/ArchiveZipFile.cpp
modules/libjar/nsZipArchive.cpp
--- a/dom/archivereader/ArchiveZipFile.cpp
+++ b/dom/archivereader/ArchiveZipFile.cpp
@@ -99,17 +99,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;
@@ -134,17 +135,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
@@ -654,28 +654,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;
 
@@ -796,17 +798,17 @@ uint32_t nsZipArchive::GetDataOffset(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 0;
 
   // -- check signature before using the structure, in case the zip file is corrupt
   ZipLocal* Local = (ZipLocal*)(data + offset);
   if ((xtolong(Local->signature) != LOCALSIG))
     return 0;
 
   //-- NOTE: extralen is different in central header and local header
@@ -825,17 +827,19 @@ MOZ_WIN_MEM_TRY_CATCH(return 0)
 //---------------------------------------------
 const uint8_t* nsZipArchive::GetData(nsZipItem* aItem)
 {
   PR_ASSERT (aItem);
 MOZ_WIN_MEM_TRY_BEGIN
   uint32_t offset = GetDataOffset(aItem);
 
   // -- check if there is enough source data in the file
-  if (!offset || offset + aItem->Size() > mFd->mLen)
+  if (!offset ||
+      mFd->mLen < aItem->Size() ||
+      offset > mFd->mLen - aItem->Size())
     return nullptr;
 
   return mFd->mFileData + offset;
 MOZ_WIN_MEM_TRY_CATCH(return nullptr)
 }
 
 // nsZipArchive::GetComment
 bool nsZipArchive::GetComment(nsACString &aComment)