Bug 1166900 - Better string length check in nsZipArchive::GetDataOffset. r+a=dveditz
--- 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)