From 811391b5e9634116b88abfdcb262986e778aa436 Mon Sep 17 00:00:00 2001 From: Keith Mok Date: Tue, 22 Mar 2016 08:52:48 -0700 Subject: Revert "Fixed comparison on 64 bit system" AOSP has a different fix. This reverts commit 65890a73d53a25b0809b22e41cdee19e3f4aa68d. Change-Id: I88fc05c791b9d5cef181e94d1cd3c40f0076f827 --- media/libstagefright/SampleTable.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'media/libstagefright/SampleTable.cpp') diff --git a/media/libstagefright/SampleTable.cpp b/media/libstagefright/SampleTable.cpp index ee5def5..93cf055 100644 --- a/media/libstagefright/SampleTable.cpp +++ b/media/libstagefright/SampleTable.cpp @@ -246,7 +246,7 @@ status_t SampleTable::setSampleToChunkParams( for (uint32_t i = 0; i < mNumSampleToChunkOffsets; ++i) { uint8_t buffer[12]; - if ((off64_t)(INT64_MAX - 8 - (i * 12)) < mSampleToChunkOffset) { + if ((off64_t)(SIZE_MAX - 8 - (i * 12)) < mSampleToChunkOffset) { return ERROR_MALFORMED; } -- cgit v1.1 From f97015ad37e8386a4215e2d25a4196edef5f7c8d Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Mon, 21 Mar 2016 09:29:06 -0700 Subject: Fix 64-bit comparison A 64-bit SIZE_MAX value cast to off64_t is always negative, causing valid files to be rejected in 64-bit mode. Change-Id: I8f61c19951f9c73292fa917081b8b2f3bfc405a0 --- media/libstagefright/SampleTable.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'media/libstagefright/SampleTable.cpp') diff --git a/media/libstagefright/SampleTable.cpp b/media/libstagefright/SampleTable.cpp index 93cf055..2f69fd8 100644 --- a/media/libstagefright/SampleTable.cpp +++ b/media/libstagefright/SampleTable.cpp @@ -209,6 +209,11 @@ status_t SampleTable::setChunkOffsetParams( status_t SampleTable::setSampleToChunkParams( off64_t data_offset, size_t data_size) { if (mSampleToChunkOffset >= 0) { + // already set + return ERROR_MALFORMED; + } + + if (data_offset < 0) { return ERROR_MALFORMED; } @@ -246,7 +251,7 @@ status_t SampleTable::setSampleToChunkParams( for (uint32_t i = 0; i < mNumSampleToChunkOffsets; ++i) { uint8_t buffer[12]; - if ((off64_t)(SIZE_MAX - 8 - (i * 12)) < mSampleToChunkOffset) { + if ((SIZE_MAX - 8 - (i * 12)) < (size_t)mSampleToChunkOffset) { return ERROR_MALFORMED; } -- cgit v1.1 From 45737cb776625f17384540523674761e6313e6d4 Mon Sep 17 00:00:00 2001 From: Zach Jang Date: Thu, 21 Apr 2016 16:10:50 -0700 Subject: Resolve merge conflict when cp'ing ag/931301 to mnc-mr1-release Change-Id: I079d1db2d30d126f8aed348bd62451acf741037d --- media/libstagefright/SampleTable.cpp | 37 +++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) (limited to 'media/libstagefright/SampleTable.cpp') diff --git a/media/libstagefright/SampleTable.cpp b/media/libstagefright/SampleTable.cpp index 97dff43..5344ae4 100644 --- a/media/libstagefright/SampleTable.cpp +++ b/media/libstagefright/SampleTable.cpp @@ -122,7 +122,7 @@ SampleTable::SampleTable(const sp &source) mDefaultSampleSize(0), mNumSampleSizes(0), mTimeToSampleCount(0), - mTimeToSample(NULL), + mTimeToSample(), mSampleTimeEntries(NULL), mCompositionTimeDeltaEntries(NULL), mNumCompositionTimeDeltaEntries(0), @@ -151,9 +151,6 @@ SampleTable::~SampleTable() { delete[] mSampleTimeEntries; mSampleTimeEntries = NULL; - delete[] mTimeToSample; - mTimeToSample = NULL; - delete mSampleIterator; mSampleIterator = NULL; } @@ -162,7 +159,7 @@ bool SampleTable::isValid() const { return mChunkOffsetOffset >= 0 && mSampleToChunkOffset >= 0 && mSampleSizeOffset >= 0 - && mTimeToSample != NULL; + && !mTimeToSample.empty(); } status_t SampleTable::setChunkOffsetParams( @@ -327,7 +324,7 @@ status_t SampleTable::setSampleSizeParams( status_t SampleTable::setTimeToSampleParams( off64_t data_offset, size_t data_size) { - if (mTimeToSample != NULL || data_size < 8) { + if (!mTimeToSample.empty() || data_size < 8) { return ERROR_MALFORMED; } @@ -343,24 +340,30 @@ status_t SampleTable::setTimeToSampleParams( } mTimeToSampleCount = U32_AT(&header[4]); - uint64_t allocSize = (uint64_t)mTimeToSampleCount * 2 * sizeof(uint32_t); - if (allocSize > UINT32_MAX) { + if ((uint64_t)mTimeToSampleCount > + (uint64_t)UINT32_MAX / (2 * sizeof(uint32_t))) { + // Choose this bound because + // 1) 2 * sizeof(uint32_t) is the amount of memory needed for one + // time-to-sample entry in the time-to-sample table. + // 2) mTimeToSampleCount is the number of entries of the time-to-sample + // table. + // 3) We hope that the table size does not exceed UINT32_MAX. + ALOGE(" Error: Time-to-sample table size too large."); + return ERROR_OUT_OF_RANGE; } - mTimeToSample = new (std::nothrow) uint32_t[mTimeToSampleCount * 2]; - if (!mTimeToSample) - return ERROR_OUT_OF_RANGE; - size_t size = sizeof(uint32_t) * mTimeToSampleCount * 2; - if (mDataSource->readAt( - data_offset + 8, mTimeToSample, size) < (ssize_t)size) { + // Note: At this point, we know that mTimeToSampleCount * 2 will not + // overflow because of the above condition. + if (!mDataSource->getVector(data_offset + 8, &mTimeToSample, + mTimeToSampleCount * 2)) { + ALOGE(" Error: Incomplete data read for time-to-sample table."); return ERROR_IO; } - for (uint32_t i = 0; i < mTimeToSampleCount * 2; ++i) { - mTimeToSample[i] = ntohl(mTimeToSample[i]); + for (size_t i = 0; i < mTimeToSample.size(); ++i) { + mTimeToSample.editItemAt(i) = ntohl(mTimeToSample[i]); } - return OK; } -- cgit v1.1 From b57b3967b1a42dd505dbe4fcf1e1d810e3ae3777 Mon Sep 17 00:00:00 2001 From: Pawin Vongmasa Date: Wed, 20 Apr 2016 15:51:48 -0700 Subject: SampleTable.cpp: Fixed a regression caused by a fix for bug 28076789. Detail: Before the original fix (Id207f369ab7b27787d83f5d8fc48dc53ed9fcdc9) for 28076789, the code allowed a time-to-sample table size to be 0. The change made in that fix disallowed such situation, which in fact should be allowed. This current patch allows it again while maintaining the security of the previous fix. Bug: 28288202 Bug: 28076789 Change-Id: I1c9a60c7f0cfcbd3d908f24998dde15d5136a295 --- media/libstagefright/SampleTable.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'media/libstagefright/SampleTable.cpp') diff --git a/media/libstagefright/SampleTable.cpp b/media/libstagefright/SampleTable.cpp index 5344ae4..8df9cb8 100644 --- a/media/libstagefright/SampleTable.cpp +++ b/media/libstagefright/SampleTable.cpp @@ -121,6 +121,7 @@ SampleTable::SampleTable(const sp &source) mSampleSizeFieldSize(0), mDefaultSampleSize(0), mNumSampleSizes(0), + mHasTimeToSample(false), mTimeToSampleCount(0), mTimeToSample(), mSampleTimeEntries(NULL), @@ -159,7 +160,7 @@ bool SampleTable::isValid() const { return mChunkOffsetOffset >= 0 && mSampleToChunkOffset >= 0 && mSampleSizeOffset >= 0 - && !mTimeToSample.empty(); + && mHasTimeToSample; } status_t SampleTable::setChunkOffsetParams( @@ -324,7 +325,7 @@ status_t SampleTable::setSampleSizeParams( status_t SampleTable::setTimeToSampleParams( off64_t data_offset, size_t data_size) { - if (!mTimeToSample.empty() || data_size < 8) { + if (mHasTimeToSample || data_size < 8) { return ERROR_MALFORMED; } @@ -364,6 +365,8 @@ status_t SampleTable::setTimeToSampleParams( for (size_t i = 0; i < mTimeToSample.size(); ++i) { mTimeToSample.editItemAt(i) = ntohl(mTimeToSample[i]); } + + mHasTimeToSample = true; return OK; } -- cgit v1.1 From 030001de8b26291b139a8c1d594f05130dafac1b Mon Sep 17 00:00:00 2001 From: Pawin Vongmasa Date: Tue, 21 Jun 2016 19:10:21 -0700 Subject: Impose a size bound for dynamically allocated tables in stbl. Impose a restriction of 200MiB for tables in stsc, stts, ctts and stss boxes. Also change mTimeToSample from Vector to array. Bug: 29367429 Change-Id: I953bea9fe0590268cf27376740f582dc88563d42 Merge conflict resolution of ag/1170200 to mnc-mr2-release --- media/libstagefright/SampleTable.cpp | 150 +++++++++++++++++++++++++++++------ 1 file changed, 126 insertions(+), 24 deletions(-) (limited to 'media/libstagefright/SampleTable.cpp') diff --git a/media/libstagefright/SampleTable.cpp b/media/libstagefright/SampleTable.cpp index 8df9cb8..bc01a2d 100644 --- a/media/libstagefright/SampleTable.cpp +++ b/media/libstagefright/SampleTable.cpp @@ -123,7 +123,7 @@ SampleTable::SampleTable(const sp &source) mNumSampleSizes(0), mHasTimeToSample(false), mTimeToSampleCount(0), - mTimeToSample(), + mTimeToSample(NULL), mSampleTimeEntries(NULL), mCompositionTimeDeltaEntries(NULL), mNumCompositionTimeDeltaEntries(0), @@ -132,7 +132,8 @@ SampleTable::SampleTable(const sp &source) mNumSyncSamples(0), mSyncSamples(NULL), mLastSyncSampleIndex(0), - mSampleToChunkEntries(NULL) { + mSampleToChunkEntries(NULL), + mTotalSize(0) { mSampleIterator = new SampleIterator(this); } @@ -143,6 +144,9 @@ SampleTable::~SampleTable() { delete[] mSyncSamples; mSyncSamples = NULL; + delete[] mTimeToSample; + mTimeToSample = NULL; + delete mCompositionDeltaLookup; mCompositionDeltaLookup = NULL; @@ -233,13 +237,43 @@ status_t SampleTable::setSampleToChunkParams( return ERROR_MALFORMED; } - if (SIZE_MAX / sizeof(SampleToChunkEntry) <= (size_t)mNumSampleToChunkOffsets) + if ((uint64_t)SIZE_MAX / sizeof(SampleToChunkEntry) <= + (uint64_t)mNumSampleToChunkOffsets) { + ALOGE("Sample-to-chunk table size too large."); return ERROR_OUT_OF_RANGE; + } + + mTotalSize += (uint64_t)mNumSampleToChunkOffsets * + sizeof(SampleToChunkEntry); + if (mTotalSize > kMaxTotalSize) { + ALOGE("Sample-to-chunk table size would make sample table too large.\n" + " Requested sample-to-chunk table size = %llu\n" + " Eventual sample table size >= %llu\n" + " Allowed sample table size = %llu\n", + (unsigned long long)mNumSampleToChunkOffsets * + sizeof(SampleToChunkEntry), + (unsigned long long)mTotalSize, + (unsigned long long)kMaxTotalSize); + return ERROR_OUT_OF_RANGE; + } mSampleToChunkEntries = new (std::nothrow) SampleToChunkEntry[mNumSampleToChunkOffsets]; - if (!mSampleToChunkEntries) + if (!mSampleToChunkEntries) { + ALOGE("Cannot allocate sample-to-chunk table with %llu entries.", + (unsigned long long)mNumSampleToChunkOffsets); return ERROR_OUT_OF_RANGE; + } + + if (mNumSampleToChunkOffsets == 0) { + return OK; + } + + if ((off64_t)(SIZE_MAX - 8 - + ((mNumSampleToChunkOffsets - 1) * sizeof(SampleToChunkEntry))) + < mSampleToChunkOffset) { + return ERROR_MALFORMED; + } for (uint32_t i = 0; i < mNumSampleToChunkOffsets; ++i) { uint8_t buffer[12]; @@ -248,8 +282,11 @@ status_t SampleTable::setSampleToChunkParams( != (ssize_t)sizeof(buffer)) { return ERROR_IO; } - - CHECK(U32_AT(buffer) >= 1); // chunk index is 1 based in the spec. + // chunk index is 1 based in the spec. + if (U32_AT(buffer) < 1) { + ALOGE("b/23534160"); + return ERROR_OUT_OF_RANGE; + } // We want the chunk index to be 0-based. mSampleToChunkEntries[i].startChunk = U32_AT(buffer) - 1; @@ -349,21 +386,41 @@ status_t SampleTable::setTimeToSampleParams( // 2) mTimeToSampleCount is the number of entries of the time-to-sample // table. // 3) We hope that the table size does not exceed UINT32_MAX. - ALOGE(" Error: Time-to-sample table size too large."); - + ALOGE("Time-to-sample table size too large."); return ERROR_OUT_OF_RANGE; } // Note: At this point, we know that mTimeToSampleCount * 2 will not // overflow because of the above condition. - if (!mDataSource->getVector(data_offset + 8, &mTimeToSample, - mTimeToSampleCount * 2)) { - ALOGE(" Error: Incomplete data read for time-to-sample table."); + + uint64_t allocSize = (uint64_t)mTimeToSampleCount * 2 * sizeof(uint32_t); + mTotalSize += allocSize; + if (mTotalSize > kMaxTotalSize) { + ALOGE("Time-to-sample table size would make sample table too large.\n" + " Requested time-to-sample table size = %llu\n" + " Eventual sample table size >= %llu\n" + " Allowed sample table size = %llu\n", + (unsigned long long)allocSize, + (unsigned long long)mTotalSize, + (unsigned long long)kMaxTotalSize); + return ERROR_OUT_OF_RANGE; + } + + mTimeToSample = new (std::nothrow) uint32_t[mTimeToSampleCount * 2]; + if (!mTimeToSample) { + ALOGE("Cannot allocate time-to-sample table with %llu entries.", + (unsigned long long)mTimeToSampleCount); + return ERROR_OUT_OF_RANGE; + } + + if (mDataSource->readAt(data_offset + 8, mTimeToSample, + (size_t)allocSize) < (ssize_t)allocSize) { + ALOGE("Incomplete data read for time-to-sample table."); return ERROR_IO; } - for (size_t i = 0; i < mTimeToSample.size(); ++i) { - mTimeToSample.editItemAt(i) = ntohl(mTimeToSample[i]); + for (size_t i = 0; i < mTimeToSampleCount * 2; ++i) { + mTimeToSample[i] = ntohl(mTimeToSample[i]); } mHasTimeToSample = true; @@ -398,17 +455,32 @@ status_t SampleTable::setCompositionTimeToSampleParams( mNumCompositionTimeDeltaEntries = numEntries; uint64_t allocSize = (uint64_t)numEntries * 2 * sizeof(uint32_t); - if (allocSize > UINT32_MAX) { + if (allocSize > SIZE_MAX) { + ALOGE("Composition-time-to-sample table size too large."); + return ERROR_OUT_OF_RANGE; + } + + mTotalSize += allocSize; + if (mTotalSize > kMaxTotalSize) { + ALOGE("Composition-time-to-sample table would make sample table too large.\n" + " Requested composition-time-to-sample table size = %llu\n" + " Eventual sample table size >= %llu\n" + " Allowed sample table size = %llu\n", + (unsigned long long)allocSize, + (unsigned long long)mTotalSize, + (unsigned long long)kMaxTotalSize); return ERROR_OUT_OF_RANGE; } mCompositionTimeDeltaEntries = new (std::nothrow) uint32_t[2 * numEntries]; - if (!mCompositionTimeDeltaEntries) + if (!mCompositionTimeDeltaEntries) { + ALOGE("Cannot allocate composition-time-to-sample table with %llu " + "entries.", (unsigned long long)numEntries); return ERROR_OUT_OF_RANGE; + } - if (mDataSource->readAt( - data_offset + 8, mCompositionTimeDeltaEntries, numEntries * 8) - < (ssize_t)numEntries * 8) { + if (mDataSource->readAt(data_offset + 8, mCompositionTimeDeltaEntries, + (size_t)allocSize) < (ssize_t)allocSize) { delete[] mCompositionTimeDeltaEntries; mCompositionTimeDeltaEntries = NULL; @@ -449,18 +521,33 @@ status_t SampleTable::setSyncSampleParams(off64_t data_offset, size_t data_size) ALOGV("Table of sync samples is empty or has only a single entry!"); } - uint64_t allocSize = mNumSyncSamples * (uint64_t)sizeof(uint32_t); + uint64_t allocSize = (uint64_t)mNumSyncSamples * sizeof(uint32_t); if (allocSize > SIZE_MAX) { + ALOGE("Sync sample table size too large."); + return ERROR_OUT_OF_RANGE; + } + + mTotalSize += allocSize; + if (mTotalSize > kMaxTotalSize) { + ALOGE("Sync sample table size would make sample table too large.\n" + " Requested sync sample table size = %llu\n" + " Eventual sample table size >= %llu\n" + " Allowed sample table size = %llu\n", + (unsigned long long)allocSize, + (unsigned long long)mTotalSize, + (unsigned long long)kMaxTotalSize); return ERROR_OUT_OF_RANGE; } mSyncSamples = new (std::nothrow) uint32_t[mNumSyncSamples]; - if (!mSyncSamples) + if (!mSyncSamples) { + ALOGE("Cannot allocate sync sample table with %llu entries.", + (unsigned long long)mNumSyncSamples); return ERROR_OUT_OF_RANGE; + } - size_t size = mNumSyncSamples * sizeof(uint32_t); - if (mDataSource->readAt(mSyncSampleOffset + 8, mSyncSamples, size) - != (ssize_t)size) { + if (mDataSource->readAt(mSyncSampleOffset + 8, mSyncSamples, + (size_t)allocSize) != (ssize_t)allocSize) { return ERROR_IO; } @@ -525,9 +612,24 @@ void SampleTable::buildSampleEntriesTable() { return; } + mTotalSize += (uint64_t)mNumSampleSizes * sizeof(SampleTimeEntry); + if (mTotalSize > kMaxTotalSize) { + ALOGE("Sample entry table size would make sample table too large.\n" + " Requested sample entry table size = %llu\n" + " Eventual sample table size >= %llu\n" + " Allowed sample table size = %llu\n", + (unsigned long long)mNumSampleSizes * sizeof(SampleTimeEntry), + (unsigned long long)mTotalSize, + (unsigned long long)kMaxTotalSize); + return; + } + mSampleTimeEntries = new (std::nothrow) SampleTimeEntry[mNumSampleSizes]; - if (!mSampleTimeEntries) + if (!mSampleTimeEntries) { + ALOGE("Cannot allocate sample entry table with %llu entries.", + (unsigned long long)mNumSampleSizes); return; + } uint32_t sampleIndex = 0; uint32_t sampleTime = 0; -- cgit v1.1 From 6679b5088f36693f5708dcaedd0c9ab7c66df27c Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Thu, 21 Jul 2016 14:43:38 +0900 Subject: DO NOT MERGE - stagefright: fix integer overflow error Bug: 30103394 Change-Id: If449d3e30a0bf2ebea5317f41813bfed094f7408 (cherry picked from commit 2c74a3cd5d1d66b9a35424b9c4443dafa6db5bef) --- media/libstagefright/SampleTable.cpp | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) (limited to 'media/libstagefright/SampleTable.cpp') diff --git a/media/libstagefright/SampleTable.cpp b/media/libstagefright/SampleTable.cpp index bc01a2d..72e30f1 100644 --- a/media/libstagefright/SampleTable.cpp +++ b/media/libstagefright/SampleTable.cpp @@ -18,6 +18,8 @@ //#define LOG_NDEBUG 0 #include +#include + #include "include/SampleTable.h" #include "include/SampleIterator.h" @@ -27,11 +29,6 @@ #include #include -/* TODO: remove after being merged into other branches */ -#ifndef UINT32_MAX -#define UINT32_MAX (4294967295U) -#endif - namespace android { // static @@ -45,6 +42,8 @@ const uint32_t SampleTable::kSampleSizeTypeCompact = FOURCC('s', 't', 'z', '2'); //////////////////////////////////////////////////////////////////////////////// +const off64_t kMaxOffset = std::numeric_limits::max(); + struct SampleTable::CompositionDeltaLookup { CompositionDeltaLookup(); @@ -233,11 +232,11 @@ status_t SampleTable::setSampleToChunkParams( mNumSampleToChunkOffsets = U32_AT(&header[4]); - if (data_size < 8 + mNumSampleToChunkOffsets * 12) { + if ((data_size - 8) / sizeof(SampleToChunkEntry) < mNumSampleToChunkOffsets) { return ERROR_MALFORMED; } - if ((uint64_t)SIZE_MAX / sizeof(SampleToChunkEntry) <= + if ((uint64_t)kMaxTotalSize / sizeof(SampleToChunkEntry) <= (uint64_t)mNumSampleToChunkOffsets) { ALOGE("Sample-to-chunk table size too large."); return ERROR_OUT_OF_RANGE; @@ -269,16 +268,19 @@ status_t SampleTable::setSampleToChunkParams( return OK; } - if ((off64_t)(SIZE_MAX - 8 - + if ((off64_t)(kMaxOffset - 8 - ((mNumSampleToChunkOffsets - 1) * sizeof(SampleToChunkEntry))) < mSampleToChunkOffset) { return ERROR_MALFORMED; } for (uint32_t i = 0; i < mNumSampleToChunkOffsets; ++i) { - uint8_t buffer[12]; + uint8_t buffer[sizeof(SampleToChunkEntry)]; + if (mDataSource->readAt( - mSampleToChunkOffset + 8 + i * 12, buffer, sizeof(buffer)) + mSampleToChunkOffset + 8 + i * sizeof(SampleToChunkEntry), + buffer, + sizeof(buffer)) != (ssize_t)sizeof(buffer)) { return ERROR_IO; } @@ -378,8 +380,7 @@ status_t SampleTable::setTimeToSampleParams( } mTimeToSampleCount = U32_AT(&header[4]); - if ((uint64_t)mTimeToSampleCount > - (uint64_t)UINT32_MAX / (2 * sizeof(uint32_t))) { + if (mTimeToSampleCount > UINT32_MAX / (2 * sizeof(uint32_t))) { // Choose this bound because // 1) 2 * sizeof(uint32_t) is the amount of memory needed for one // time-to-sample entry in the time-to-sample table. @@ -455,7 +456,7 @@ status_t SampleTable::setCompositionTimeToSampleParams( mNumCompositionTimeDeltaEntries = numEntries; uint64_t allocSize = (uint64_t)numEntries * 2 * sizeof(uint32_t); - if (allocSize > SIZE_MAX) { + if (allocSize > kMaxTotalSize) { ALOGE("Composition-time-to-sample table size too large."); return ERROR_OUT_OF_RANGE; } @@ -522,7 +523,7 @@ status_t SampleTable::setSyncSampleParams(off64_t data_offset, size_t data_size) } uint64_t allocSize = (uint64_t)mNumSyncSamples * sizeof(uint32_t); - if (allocSize > SIZE_MAX) { + if (allocSize > kMaxTotalSize) { ALOGE("Sync sample table size too large."); return ERROR_OUT_OF_RANGE; } -- cgit v1.1