Bug 1439433 - Length check FileBlockCache::mBlockChanges access. r=gerald draft
authorChris Pearce <cpearce@mozilla.com>
Thu, 22 Feb 2018 15:00:18 +1300
changeset 758241 b6ada7bfb755ae285f0010bd6eff2e305fc5fbf0
parent 757574 861067332bac96a44bbf41ef366f58a30476057b
push id99999
push userbmo:cpearce@mozilla.com
push dateThu, 22 Feb 2018 02:54:17 +0000
reviewersgerald
bugs1439433
milestone60.0a1
Bug 1439433 - Length check FileBlockCache::mBlockChanges access. r=gerald I can't for the life of me figure out how we get into the situation where the block change list is empty here, or how we can get past some of the existing null checks in the code, but we can at least add some more checks to hopefully ensure we don't crash... MozReview-Commit-ID: 168G94IyrWt
dom/media/FileBlockCache.cpp
--- a/dom/media/FileBlockCache.cpp
+++ b/dom/media/FileBlockCache.cpp
@@ -449,31 +449,34 @@ nsresult FileBlockCache::Read(int64_t aO
   while (bytesToRead > 0) {
     int32_t blockIndex = static_cast<int32_t>(offset / BLOCK_SIZE);
     int32_t start = offset % BLOCK_SIZE;
     int32_t amount = std::min(BLOCK_SIZE - start, bytesToRead);
 
     // If the block is not yet written to file, we can just read from
     // the memory buffer, otherwise we need to read from file.
     int32_t bytesRead = 0;
-    RefPtr<BlockChange> change = mBlockChanges[blockIndex];
+    MOZ_ASSERT(!mBlockChanges.IsEmpty());
+    MOZ_ASSERT(blockIndex >= 0 &&
+               static_cast<uint32_t>(blockIndex) < mBlockChanges.Length());
+    RefPtr<BlockChange> change = mBlockChanges.SafeElementAt(blockIndex);
     if (change && change->IsWrite()) {
       // Block isn't yet written to file. Read from memory buffer.
       const uint8_t* blockData = change->mData.get();
       memcpy(dst, blockData + start, amount);
       bytesRead = amount;
     } else {
       if (change && change->IsMove()) {
         // The target block is the destination of a not-yet-completed move
         // action, so read from the move's source block from file. Note we
         // *don't* follow a chain of moves here, as a move's source index
         // is resolved when MoveBlock() is called, and the move's source's
         // block could be have itself been subject to a move (or write)
         // which happened *after* this move was recorded.
-        blockIndex = mBlockChanges[blockIndex]->mSourceBlockIndex;
+        blockIndex = change->mSourceBlockIndex;
       }
       // Block has been written to file, either as the source block of a move,
       // or as a stable (all changes made) block. Read the data directly
       // from file.
       nsresult res;
       {
         MutexAutoUnlock unlock(mDataMutex);
         MutexAutoLock lock(mFileMutex);