Bug 1384243 - Sanitize offset inputs in MediaResourceIndex - r=cpearce a=gchang
authorGerald Squelart <gsquelart@mozilla.com>
Tue, 01 Aug 2017 14:07:55 +1200
changeset 423439 081973e3343d57038ae3020d5779f27f34145907
parent 423438 9bc2711015128804682ade612b929a1989ca8ada
child 423440 1058d41cdddf4897fc99e8abce316a51582e894b
push id1517
push userjlorenzo@mozilla.com
push dateThu, 14 Sep 2017 16:50:54 +0000
treeherdermozilla-release@3b41fd564418 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce, gchang
bugs1384243
milestone56.0
Bug 1384243 - Sanitize offset inputs in MediaResourceIndex - r=cpearce a=gchang Also check that the offset doesn't overflow during reads. MozReview-Commit-ID: DT5neeZuMZu
dom/media/MediaResource.cpp
dom/media/MediaResource.h
--- a/dom/media/MediaResource.cpp
+++ b/dom/media/MediaResource.cpp
@@ -1564,16 +1564,20 @@ MediaResourceIndex::Read(char* aBuffer, 
   // We purposefuly don't check that we may attempt to read past
   // mResource->GetLength() as the resource's length may change over time.
 
   nsresult rv = ReadAt(mOffset, aBuffer, aCount, aBytes);
   if (NS_FAILED(rv)) {
     return rv;
   }
   mOffset += *aBytes;
+  if (mOffset < 0) {
+    // Very unlikely overflow; just return to position 0.
+    mOffset = 0;
+  }
   return NS_OK;
 }
 
 static nsCString
 ResultName(nsresult aResult)
 {
   nsCString name;
   GetErrorName(aResult, name);
@@ -1592,16 +1596,20 @@ MediaResourceIndex::ReadAt(int64_t aOffs
 
   *aBytes = 0;
 
   if (aCount == 0) {
     return NS_OK;
   }
 
   const int64_t endOffset = aOffset + aCount;
+  if (aOffset < 0 || endOffset < aOffset) {
+    return NS_ERROR_ILLEGAL_VALUE;
+  }
+
   const int64_t lastBlockOffset = CacheOffsetContaining(endOffset - 1);
 
   if (mCachedBytes != 0 && mCachedOffset + mCachedBytes >= aOffset &&
       mCachedOffset < endOffset) {
     // There is data in the cache that is not completely before aOffset and not
     // completely after endOffset, so it could be usable (with potential top-up).
     if (aOffset < mCachedOffset) {
       // We need to read before the cached data.
@@ -1904,47 +1912,57 @@ MediaResourceIndex::CacheOrReadAt(int64_
 
 nsresult
 MediaResourceIndex::UncachedReadAt(int64_t aOffset,
                                    char* aBuffer,
                                    uint32_t aCount,
                                    uint32_t* aBytes) const
 {
   *aBytes = 0;
+  if (aOffset < 0) {
+    return NS_ERROR_ILLEGAL_VALUE;
+  }
   if (aCount != 0) {
     for (;;) {
       uint32_t bytesRead = 0;
       nsresult rv = mResource->ReadAt(aOffset, aBuffer, aCount, &bytesRead);
       if (NS_FAILED(rv)) {
         return rv;
       }
       if (bytesRead == 0) {
         break;
       }
       *aBytes += bytesRead;
       aCount -= bytesRead;
       if (aCount == 0) {
         break;
       }
       aOffset += bytesRead;
+      if (aOffset < 0) {
+        // Very unlikely overflow.
+        return NS_ERROR_FAILURE;
+      }
       aBuffer += bytesRead;
     }
   }
   return NS_OK;
 }
 
 nsresult
 MediaResourceIndex::UncachedRangedReadAt(int64_t aOffset,
                                          char* aBuffer,
                                          uint32_t aRequestedCount,
                                          uint32_t aExtraCount,
                                          uint32_t* aBytes) const
 {
   *aBytes = 0;
   uint32_t count = aRequestedCount + aExtraCount;
+  if (aOffset < 0 || count < aRequestedCount) {
+    return NS_ERROR_ILLEGAL_VALUE;
+  }
   if (count != 0) {
     for (;;) {
       uint32_t bytesRead = 0;
       nsresult rv = mResource->ReadAt(aOffset, aBuffer, count, &bytesRead);
       if (NS_FAILED(rv)) {
         return rv;
       }
       if (bytesRead == 0) {
@@ -1952,16 +1970,20 @@ MediaResourceIndex::UncachedRangedReadAt
       }
       *aBytes += bytesRead;
       count -= bytesRead;
       if (count <= aExtraCount) {
         // We have read at least aRequestedCount, don't loop anymore.
         break;
       }
       aOffset += bytesRead;
+      if (aOffset < 0) {
+        // Very unlikely overflow.
+        return NS_ERROR_FAILURE;
+      }
       aBuffer += bytesRead;
     }
   }
   return NS_OK;
 }
 
 nsresult
 MediaResourceIndex::Seek(int32_t aWhence, int64_t aOffset)
@@ -1980,16 +2002,19 @@ MediaResourceIndex::Seek(int32_t aWhence
       }
       aOffset = mResource->GetLength() - aOffset;
     }
       break;
     default:
       return NS_ERROR_FAILURE;
   }
 
+  if (aOffset < 0) {
+    return NS_ERROR_ILLEGAL_VALUE;
+  }
   mOffset = aOffset;
 
   return NS_OK;
 }
 
 } // namespace mozilla
 
 // avoid redefined macro in unified build
--- a/dom/media/MediaResource.h
+++ b/dom/media/MediaResource.h
@@ -755,28 +755,35 @@ public:
   // This method returns nullptr if anything fails.
   // Otherwise, it returns an owned buffer.
   // MediaReadAt may return fewer bytes than requested if end of stream is
   // encountered. There is no need to call it again to get more data.
   // Note this method will not update mOffset.
   already_AddRefed<MediaByteBuffer> MediaReadAt(int64_t aOffset, uint32_t aCount) const
   {
     RefPtr<MediaByteBuffer> bytes = new MediaByteBuffer();
+    if (aOffset < 0) {
+      return bytes.forget();
+    }
     bool ok = bytes->SetLength(aCount, fallible);
     NS_ENSURE_TRUE(ok, nullptr);
     char* curr = reinterpret_cast<char*>(bytes->Elements());
     const char* start = curr;
     while (aCount > 0) {
       uint32_t bytesRead;
       nsresult rv = mResource->ReadAt(aOffset, curr, aCount, &bytesRead);
       NS_ENSURE_SUCCESS(rv, nullptr);
       if (!bytesRead) {
         break;
       }
       aOffset += bytesRead;
+      if (aOffset < 0) {
+        // Very unlikely overflow.
+        break;
+      }
       aCount -= bytesRead;
       curr += bytesRead;
     }
     bytes->SetLength(curr - start);
     return bytes.forget();
   }
   // Get the length of the stream in bytes. Returns -1 if not known.
   // This can change over time; after a seek operation, a misbehaving