From 582c02ea5c9c8db5f993d784a0a85b275b2e59fd Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Tue, 23 Feb 2016 14:48:46 -0800 Subject: Also fix out of bounds access for normal read Previous fix accidentally only fixed the fragmented read case. Bug: 27208621 Change-Id: Ie16f1920b84c8aba613842659238fcd5925694ad --- media/libstagefright/MPEG4Extractor.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'media/libstagefright/MPEG4Extractor.cpp') diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp index e4f8384..f8789da 100755 --- a/media/libstagefright/MPEG4Extractor.cpp +++ b/media/libstagefright/MPEG4Extractor.cpp @@ -4228,7 +4228,15 @@ status_t MPEG4Source::read( continue; } - CHECK(dstOffset + 4 <= mBuffer->size()); + if (dstOffset > SIZE_MAX - 4 || + dstOffset + 4 > SIZE_MAX - nalLength || + dstOffset + 4 + nalLength > mBuffer->size()) { + ALOGE("b/27208621 : %zu %zu", dstOffset, mBuffer->size()); + android_errorWriteLog(0x534e4554, "27208621"); + mBuffer->release(); + mBuffer = NULL; + return ERROR_MALFORMED; + } dstData[dstOffset++] = 0; dstData[dstOffset++] = 0; -- cgit v1.1 From b7c8681b7f3c093b1c0f667007c490d9c563655c Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Tue, 23 Feb 2016 14:48:46 -0800 Subject: Also fix out of bounds access for normal read Previous fix accidentally only fixed the fragmented read case. Bug: 27208621 Change-Id: Ie16f1920b84c8aba613842659238fcd5925694ad --- media/libstagefright/MPEG4Extractor.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'media/libstagefright/MPEG4Extractor.cpp') diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp index e4f8384..f8789da 100755 --- a/media/libstagefright/MPEG4Extractor.cpp +++ b/media/libstagefright/MPEG4Extractor.cpp @@ -4228,7 +4228,15 @@ status_t MPEG4Source::read( continue; } - CHECK(dstOffset + 4 <= mBuffer->size()); + if (dstOffset > SIZE_MAX - 4 || + dstOffset + 4 > SIZE_MAX - nalLength || + dstOffset + 4 + nalLength > mBuffer->size()) { + ALOGE("b/27208621 : %zu %zu", dstOffset, mBuffer->size()); + android_errorWriteLog(0x534e4554, "27208621"); + mBuffer->release(); + mBuffer = NULL; + return ERROR_MALFORMED; + } dstData[dstOffset++] = 0; dstData[dstOffset++] = 0; -- cgit v1.1 From c27fc358c91ed67965243294387dfb5ce7cd7fbf Mon Sep 17 00:00:00 2001 From: "Joshua J. Drake" Date: Thu, 9 Apr 2015 00:46:42 -0500 Subject: MPEG4Extractor: still more NULL dereference fixes When processing various FourCC values within MP4 media, mLastTrack is accessed without first ensuring that a track has been encoutered. Check for NULL and bail out instead of crashing. Bug: 20139950 Change-Id: Ie16687024d17348f576a0e13bd60bd4d6898de91 --- media/libstagefright/MPEG4Extractor.cpp | 3 +++ 1 file changed, 3 insertions(+) (limited to 'media/libstagefright/MPEG4Extractor.cpp') diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp index c7c238e..2d2e8fb 100755 --- a/media/libstagefright/MPEG4Extractor.cpp +++ b/media/libstagefright/MPEG4Extractor.cpp @@ -1980,6 +1980,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) { return ERROR_IO; } + if (mLastTrack == NULL) + return ERROR_MALFORMED; + uint32_t type = ntohl(buffer); // For the 3GPP file format, the handler-type within the 'hdlr' box // shall be 'text'. We also want to support 'sbtl' handler type -- cgit v1.1 From c4795f0ab03bc1188f2b6ca25b333b8a7220daf3 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Wed, 3 Feb 2016 14:28:00 -0800 Subject: MPEG4Extractor: cast media time to int64_t in order to avoid check on unsigned integer overflow. Change-Id: Iad5ae41f0bbfc5e837b4b78e8acaa3f9462329e6 --- media/libstagefright/MPEG4Extractor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'media/libstagefright/MPEG4Extractor.cpp') diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp index 2d2e8fb..c056a25 100755 --- a/media/libstagefright/MPEG4Extractor.cpp +++ b/media/libstagefright/MPEG4Extractor.cpp @@ -1032,7 +1032,7 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) { int64_t delay = (media_time * samplerate + 500000) / 1000000; mLastTrack->meta->setInt32(kKeyEncoderDelay, delay); - int64_t paddingus = duration - (segment_duration + media_time); + int64_t paddingus = duration - (int64_t)(segment_duration + media_time); if (paddingus < 0) { // track duration from media header (which is what kKeyDuration is) might // be slightly shorter than the segment duration, which would make the -- cgit v1.1 From e7142a0703bc93f75e213e96ebc19000022afed9 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Wed, 11 May 2016 11:11:20 -0700 Subject: Check malloc result to avoid NPD Bug: 28471206 Change-Id: Id5d055d76893d6f53a2e524ff5f282d1ddca3345 --- media/libstagefright/MPEG4Extractor.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'media/libstagefright/MPEG4Extractor.cpp') diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp index f8789da..f6206d2 100755 --- a/media/libstagefright/MPEG4Extractor.cpp +++ b/media/libstagefright/MPEG4Extractor.cpp @@ -535,6 +535,10 @@ status_t MPEG4Extractor::readMetaData() { } if (psshsize > 0 && psshsize <= UINT32_MAX) { char *buf = (char*)malloc(psshsize); + if (!buf) { + ALOGE("b/28471206"); + return NO_MEMORY; + } char *ptr = buf; for (size_t i = 0; i < mPssh.size(); i++) { memcpy(ptr, mPssh[i].uuid, 20); // uuid + length @@ -1702,6 +1706,11 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) { sp buffer = new ABuffer(chunk_data_size); + if (buffer->data() == NULL) { + ALOGE("b/28471206"); + return NO_MEMORY; + } + if (mDataSource->readAt( data_offset, buffer->data(), chunk_data_size) < chunk_data_size) { return ERROR_IO; @@ -1719,6 +1728,11 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) { { sp buffer = new ABuffer(chunk_data_size); + if (buffer->data() == NULL) { + ALOGE("b/28471206"); + return NO_MEMORY; + } + if (mDataSource->readAt( data_offset, buffer->data(), chunk_data_size) < chunk_data_size) { return ERROR_IO; @@ -2051,6 +2065,10 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) { return ERROR_MALFORMED; } sp buffer = new ABuffer(chunk_data_size + 1); + if (buffer->data() == NULL) { + ALOGE("b/28471206"); + return NO_MEMORY; + } if (mDataSource->readAt( data_offset, buffer->data(), chunk_data_size) != (ssize_t)chunk_data_size) { return ERROR_IO; -- cgit v1.1 From f81038006b4c59a5a148dcad887371206033c28f Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Fri, 28 Aug 2015 10:35:35 -0700 Subject: MPEG4Extractor: ensure kKeyTrackID exists before creating an MPEG4Source as track. GenericSource: return error when no track exists. SampleIterator: make sure mSamplesPerChunk is not zero before using it as divisor. Bug: 21657957 Bug: 23705695 Bug: 22802344 Bug: 28799341 Change-Id: I7664992ade90b935d3f255dcd43ecc2898f30b04 (cherry picked from commit 0386c91b8a910a134e5898ffa924c1b6c7560b13) --- media/libstagefright/MPEG4Extractor.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'media/libstagefright/MPEG4Extractor.cpp') diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp index f6206d2..4c10cc9 100755 --- a/media/libstagefright/MPEG4Extractor.cpp +++ b/media/libstagefright/MPEG4Extractor.cpp @@ -933,6 +933,11 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) { } if (isTrack) { + int32_t trackId; + // There must be exact one track header per track. + if (!mLastTrack->meta->findInt32(kKeyTrackID, &trackId)) { + mLastTrack->skipTrack = true; + } if (mLastTrack->skipTrack) { Track *cur = mFirstTrack; @@ -2869,6 +2874,9 @@ sp MPEG4Extractor::getTrack(size_t index) { break; } } + } else { + ALOGE("b/21657957"); + return NULL; } ALOGV("getTrack called, pssh: %zu", mPssh.size()); -- cgit v1.1 From b5203aba00dc60bee526d78e5851f0a34c4b5bd7 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Mon, 19 Sep 2016 16:22:56 -0700 Subject: Limit mp4 atom size to something reasonable Bug: 28615448 Change-Id: I5916f6839b4a9bbee4388a106e7373bcd4154f5a (cherry picked from commit cb898dca47ac03738db91ddc371207435d2a1526) --- media/libstagefright/MPEG4Extractor.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'media/libstagefright/MPEG4Extractor.cpp') diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp index 4c10cc9..9e7f298 100755 --- a/media/libstagefright/MPEG4Extractor.cpp +++ b/media/libstagefright/MPEG4Extractor.cpp @@ -50,6 +50,12 @@ namespace android { +enum { + // maximum size of an atom. Some atoms can be bigger according to the spec, + // but we only allow up to this size. + kMaxAtomSize = 64 * 1024 * 1024, +}; + class MPEG4Source : public MediaSource { public: // Caller retains ownership of both "dataSource" and "sampleTable". @@ -836,6 +842,13 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) { PathAdder autoAdder(&mPath, chunk_type); off64_t chunk_data_size = *offset + chunk_size - data_offset; + if (chunk_type != FOURCC('m', 'd', 'a', 't') && chunk_data_size > kMaxAtomSize) { + char errMsg[100]; + sprintf(errMsg, "%s atom has size %" PRId64, chunk, chunk_data_size); + ALOGE("%s (b/28615448)", errMsg); + android_errorWriteWithInfoLog(0x534e4554, "28615448", -1, errMsg, strlen(errMsg)); + return ERROR_MALFORMED; + } if (chunk_type != FOURCC('c', 'p', 'r', 't') && chunk_type != FOURCC('c', 'o', 'v', 'r') -- cgit v1.1