Bug 1384243 - Sanitize offset inputs in MediaResourceIndex - r?cpearce
Also check that the offset doesn't overflow during reads.
MozReview-Commit-ID: DT5neeZuMZu
--- 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