From 22bc2ae4c037b78972caf56a4aee851725b1cdec Mon Sep 17 00:00:00 2001
From: "Joshua J. Drake" <android-open-source@qoop.org>
Date: Thu, 9 Apr 2015 00:46:42 -0500
Subject: [PATCH 1/7] MPEG4Extractor: still more NULL derefernce 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.

Change-Id: I3b86377030d73b3134b8769c590509c4f23d9f19
---
media/libstagefright/MPEG4Extractor.cpp | 56 +++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)

diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp
index 8bf7f63..9098838 100644
--- a/media/libstagefright/MPEG4Extractor.cpp
+++ b/media/libstagefright/MPEG4Extractor.cpp
@@ -878,6 +878,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
}
}

+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
mLastTrack->sampleTable = new SampleTable(mDataSource);
}

@@ -1032,6 +1035,10 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
}
original_fourcc = ntohl(original_fourcc);
ALOGV("read original format: %d", original_fourcc);
+
+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
mLastTrack->meta->setCString(kKeyMIMEType, FourCC2MIME(original_fourcc));
uint32_t num_channels = 0;
uint32_t sample_rate = 0;
@@ -1087,6 +1094,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
return ERROR_IO;
}

+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
mLastTrack->meta->setInt32(kKeyCryptoMode, defaultAlgorithmId);
mLastTrack->meta->setInt32(kKeyCryptoDefaultIVSize, defaultIVSize);
mLastTrack->meta->setData(kKeyCryptoKey, 'tenc', defaultKeyId, 16);
@@ -1261,6 +1271,10 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
// display the timed text.
// For encrypted files, there may also be more than one entry.
const char *mime;
+
+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
CHECK(mLastTrack->meta->findCString(kKeyMIMEType, &mime));
if (strcasecmp(mime, MEDIA_MIMETYPE_TEXT_3GPP) &&
strcasecmp(mime, "application/octet-stream")) {
@@ -1307,6 +1321,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
uint16_t sample_size = U16_AT(&buffer[18]);
uint32_t sample_rate = U32_AT(&buffer[24]) >> 16;

+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
if (chunk_type != FOURCC('e', 'n', 'c', 'a')) {
// if the chunk type is enca, we'll get the type from the sinf/frma box later
mLastTrack->meta->setCString(kKeyMIMEType, FourCC2MIME(chunk_type));
@@ -1368,6 +1385,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
// printf("*** coding='%s' width=%d height=%d\n",
// chunk, width, height);

+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
if (chunk_type != FOURCC('e', 'n', 'c', 'v')) {
// if the chunk type is encv, we'll get the type from the sinf/frma box later
mLastTrack->meta->setCString(kKeyMIMEType, FourCC2MIME(chunk_type));
@@ -1595,6 +1615,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
return ERROR_MALFORMED;
}

+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
mLastTrack->meta->setData(
kKeyESDS, kTypeESDS, &buffer[4], chunk_data_size - 4);

@@ -1627,6 +1650,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
return ERROR_IO;
}

+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
mLastTrack->meta->setData(
kKeyAVCC, kTypeAVCC, buffer->data(), chunk_data_size);

@@ -1641,6 +1667,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
return ERROR_IO;
}

+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
mLastTrack->meta->setData(
kKeyHVCC, kTypeHVCC, buffer->data(), chunk_data_size);

@@ -1674,6 +1703,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
return ERROR_IO;
}

+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
mLastTrack->meta->setData(kKeyD263, kTypeD263, buffer, chunk_data_size);

break;
@@ -1852,6 +1884,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
return ERROR_IO;
}

+ if (!mLastTrack)
+ 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
@@ -1884,6 +1919,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {

case FOURCC('t', 'x', '3', 'g'):
{
+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
uint32_t type;
const void *data;
size_t size = 0;
@@ -2108,6 +2146,9 @@ status_t MPEG4Extractor::parseSegmentIndex(off64_t offset, size_t size) {

uint64_t sidxDuration = total_duration * 1000000 / timeScale;

+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
int64_t metaDuration;
if (!mLastTrack->meta->findInt64(kKeyDuration, &metaDuration) || metaDuration == 0) {
mLastTrack->meta->setInt64(kKeyDuration, sidxDuration);
@@ -2158,6 +2199,9 @@ status_t MPEG4Extractor::parseTrackHeader(
return ERROR_UNSUPPORTED;
}

+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
mLastTrack->meta->setInt32(kKeyTrackID, id);

size_t matrixOffset = dynSize + 16;
@@ -2340,6 +2384,9 @@ status_t MPEG4Extractor::parseITunesMetaData(off64_t offset, size_t size) {
int32_t delay, padding;
if (sscanf(mLastCommentData,
" %*x %x %x %*x", &delay, &padding) == 2) {
+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
mLastTrack->meta->setInt32(kKeyEncoderDelay, delay);
mLastTrack->meta->setInt32(kKeyEncoderPadding, padding);
}
@@ -2702,6 +2749,9 @@ status_t MPEG4Extractor::updateAudioTrackInfoFromESDS_MPEG4Audio(

if (objectTypeIndication == 0xe1) {
// This isn't MPEG4 audio at all, it's QCELP 14k...
+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
mLastTrack->meta->setCString(kKeyMIMEType, MEDIA_MIMETYPE_AUDIO_QCELP);
return OK;
}
@@ -2750,6 +2800,9 @@ status_t MPEG4Extractor::updateAudioTrackInfoFromESDS_MPEG4Audio(
objectType = 32 + br.getBits(6);
}

+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
//keep AOT type
mLastTrack->meta->setInt32(kKeyAACAOT, objectType);

@@ -2920,6 +2973,9 @@ status_t MPEG4Extractor::updateAudioTrackInfoFromESDS_MPEG4Audio(
return ERROR_UNSUPPORTED;
}

+ if (!mLastTrack)
+ return ERROR_MALFORMED;
+
int32_t prevSampleRate;
CHECK(mLastTrack->meta->findInt32(kKeySampleRate, &prevSampleRate));

--
1.9.1

From cb98d3e28637225816913e538124b099a2be5da8 Mon Sep 17 00:00:00 2001
From: "Joshua J. Drake" <android-open-source@qoop.org>
Date: Wed, 8 Apr 2015 22:21:53 -0500
Subject: [PATCH 2/7] Fix null-pointer-dereferences accessing the SampleTable

While processing various sample table related FourCC values, methods are called
on a NULL mLastTrack or sampleTable object. This leads to undefined behavior
which typically results in a crash (denial of service condition).

Change-Id: I39a894f8709d9937a0456ae5b3a201f7ecf12ed0
---
media/libstagefright/MPEG4Extractor.cpp | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp
index 9098838..47b267f 100644
--- a/media/libstagefright/MPEG4Extractor.cpp
+++ b/media/libstagefright/MPEG4Extractor.cpp
@@ -1413,6 +1413,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
case FOURCC('s', 't', 'c', 'o'):
case FOURCC('c', 'o', '6', '4'):
{
+ if (!mLastTrack || !mLastTrack->sampleTable.get())
+ return ERROR_MALFORMED;
+
status_t err =
mLastTrack->sampleTable->setChunkOffsetParams(
chunk_type, data_offset, chunk_data_size);
@@ -1428,6 +1431,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {

case FOURCC('s', 't', 's', 'c'):
{
+ if (!mLastTrack || !mLastTrack->sampleTable.get())
+ return ERROR_MALFORMED;
+
status_t err =
mLastTrack->sampleTable->setSampleToChunkParams(
data_offset, chunk_data_size);
@@ -1444,6 +1450,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
case FOURCC('s', 't', 's', 'z'):
case FOURCC('s', 't', 'z', '2'):
{
+ if (!mLastTrack || !mLastTrack->sampleTable.get())
+ return ERROR_MALFORMED;
+
status_t err =
mLastTrack->sampleTable->setSampleSizeParams(
chunk_type, data_offset, chunk_data_size);
@@ -1513,6 +1522,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {

case FOURCC('s', 't', 't', 's'):
{
+ if (!mLastTrack || !mLastTrack->sampleTable.get())
+ return ERROR_MALFORMED;
+
*offset += chunk_size;

status_t err =
@@ -1528,6 +1540,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {

case FOURCC('c', 't', 't', 's'):
{
+ if (!mLastTrack || !mLastTrack->sampleTable.get())
+ return ERROR_MALFORMED;
+
*offset += chunk_size;

status_t err =
@@ -1543,6 +1558,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {

case FOURCC('s', 't', 's', 's'):
{
+ if (!mLastTrack || !mLastTrack->sampleTable.get())
+ return ERROR_MALFORMED;
+
*offset += chunk_size;

status_t err =
--
1.9.1

From e79eaf489ad7fa840cb3fb4b5dfa9cdb8e4fe7f0 Mon Sep 17 00:00:00 2001
From: "Joshua J. Drake" <android-open-source@qoop.org>
Date: Wed, 8 Apr 2015 23:13:02 -0500
Subject: [PATCH 3/7] Fix multiple division-by-zero conditions in MPEG4 parsing

Several situations arise processing MP4 atoms that lead to undefined behavior
when dividing by zero. Typically this results in a crash (denial of service
condition).

NOTE: In most cases we simply avoid the division, leaving kKeyDuration unset.
It may be more desirable to bail out, as we do in the parseSegmentIndex case.

Change-Id: Iab0118934b49eabd0b44be8408160d7f122c28df
---
media/libstagefright/MPEG4Extractor.cpp | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp
index 47b267f..44218ee 100644
--- a/media/libstagefright/MPEG4Extractor.cpp
+++ b/media/libstagefright/MPEG4Extractor.cpp
@@ -1207,7 +1207,7 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
duration = ntohl(duration32);
}
}
- if (duration != 0) {
+ if (duration != 0 && mLastTrack->timescale != 0) {
mLastTrack->meta->setInt64(
kKeyDuration, (duration * 1000000) / mLastTrack->timescale);
}
@@ -1821,7 +1821,7 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
}
duration = d32;
}
- if (duration != 0) {
+ if (duration != 0 && mHeaderTimescale != 0) {
mFileMetaData->setInt64(kKeyDuration, duration * 1000000 / mHeaderTimescale);
}

@@ -1870,7 +1870,7 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
return ERROR_MALFORMED;
}

- if (duration != 0) {
+ if (duration != 0 && mHeaderTimescale != 0) {
mFileMetaData->setInt64(kKeyDuration, duration * 1000000 / mHeaderTimescale);
}

@@ -2080,6 +2080,8 @@ status_t MPEG4Extractor::parseSegmentIndex(off64_t offset, size_t size) {
if (!mDataSource->getUInt32(offset + 8, &timeScale)) {
return ERROR_MALFORMED;
}
+ if (timeScale < 1)
+ return ERROR_MALFORMED;
ALOGV("sidx refid/timescale: %d/%d", referenceId, timeScale);

uint64_t earliestPresentationTime;
--
1.9.1

From 9ad4ad1fce9ee2f2c9d431c7bfc26dd26b1372f8 Mon Sep 17 00:00:00 2001
From: "Joshua J. Drake" <android-open-source@qoop.org>
Date: Wed, 8 Apr 2015 23:23:55 -0500
Subject: [PATCH 4/7] Fix several ineffective integer overflow checks

Commit edd4a76 (which addressed bugs 15328708, 15342615, 15342751) added
several integer overflow checks. Unfortunately, those checks fail to take into
account integer promotion rules and are thus themselves subject to an integer
overflow. Cast the sizeof() operator to a uint64_t to force promotion while
multiplying.

Change-Id: I2e70584ab566dbaa2fba4df6ca7a89b348ae9a06
---
media/libstagefright/SampleTable.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/media/libstagefright/SampleTable.cpp b/media/libstagefright/SampleTable.cpp
index bdd6d56..b572af3 100644
--- a/media/libstagefright/SampleTable.cpp
+++ b/media/libstagefright/SampleTable.cpp
@@ -330,7 +330,7 @@ status_t SampleTable::setTimeToSampleParams(
}

mTimeToSampleCount = U32_AT(&header[4]);
- uint64_t allocSize = mTimeToSampleCount * 2 * sizeof(uint32_t);
+ uint64_t allocSize = mTimeToSampleCount * 2 * (uint64_t)sizeof(uint32_t);
if (allocSize > SIZE_MAX) {
return ERROR_OUT_OF_RANGE;
}
@@ -376,7 +376,7 @@ status_t SampleTable::setCompositionTimeToSampleParams(
}

mNumCompositionTimeDeltaEntries = numEntries;
- uint64_t allocSize = numEntries * 2 * sizeof(uint32_t);
+ uint64_t allocSize = numEntries * 2 * (uint64_t)sizeof(uint32_t);
if (allocSize > SIZE_MAX) {
return ERROR_OUT_OF_RANGE;
}
@@ -426,7 +426,7 @@ 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 * sizeof(uint32_t);
+ uint64_t allocSize = mNumSyncSamples * (uint64_t)sizeof(uint32_t);
if (allocSize > SIZE_MAX) {
return ERROR_OUT_OF_RANGE;
}
--
1.9.1

From bdccd0d0c1efbe8be52acf076e0421ae1f2ff257 Mon Sep 17 00:00:00 2001
From: "Joshua J. Drake" <android-open-source@qoop.org>
Date: Wed, 8 Apr 2015 23:31:25 -0500
Subject: [PATCH 5/7] Detect allocation failures and bail gracefully

During the processing of several sample table related MP4 atoms, allocation
sizes could be large enough cause a std::bad_alloc exception to be raised. This
typically causes a crash (denial of service condition). Use std::nothrow to
catch allocation failures and return gracefully.

Change-Id: Id70546c9a9d7a1af58ccbf732b000246bc6bb22e
---
media/libstagefright/SampleTable.cpp | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/media/libstagefright/SampleTable.cpp b/media/libstagefright/SampleTable.cpp
index b572af3..bfae474 100644
--- a/media/libstagefright/SampleTable.cpp
+++ b/media/libstagefright/SampleTable.cpp
@@ -231,7 +231,9 @@ status_t SampleTable::setSampleToChunkParams(
}

mSampleToChunkEntries =
- new SampleToChunkEntry[mNumSampleToChunkOffsets];
+ new (std::nothrow) SampleToChunkEntry[mNumSampleToChunkOffsets];
+ if (!mSampleToChunkEntries)
+ return ERROR_OUT_OF_RANGE;

for (uint32_t i = 0; i < mNumSampleToChunkOffsets; ++i) {
uint8_t buffer[12];
@@ -334,7 +336,9 @@ status_t SampleTable::setTimeToSampleParams(
if (allocSize > SIZE_MAX) {
return ERROR_OUT_OF_RANGE;
}
- mTimeToSample = new uint32_t[mTimeToSampleCount * 2];
+ 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(
@@ -381,7 +385,9 @@ status_t SampleTable::setCompositionTimeToSampleParams(
return ERROR_OUT_OF_RANGE;
}

- mCompositionTimeDeltaEntries = new uint32_t[2 * numEntries];
+ mCompositionTimeDeltaEntries = new (std::nothrow) uint32_t[2 * numEntries];
+ if (!mCompositionTimeDeltaEntries)
+ return ERROR_OUT_OF_RANGE;

if (mDataSource->readAt(
data_offset + 8, mCompositionTimeDeltaEntries, numEntries * 8)
@@ -431,7 +437,10 @@ status_t SampleTable::setSyncSampleParams(off64_t data_offset, size_t data_size)
return ERROR_OUT_OF_RANGE;
}

- mSyncSamples = new uint32_t[mNumSyncSamples];
+ mSyncSamples = new (std::nothrow) uint32_t[mNumSyncSamples];
+ if (!mSyncSamples)
+ return ERROR_OUT_OF_RANGE;
+
size_t size = mNumSyncSamples * sizeof(uint32_t);
if (mDataSource->readAt(mSyncSampleOffset + 8, mSyncSamples, size)
!= (ssize_t)size) {
@@ -499,7 +508,9 @@ void SampleTable::buildSampleEntriesTable() {
return;
}

- mSampleTimeEntries = new SampleTimeEntry[mNumSampleSizes];
+ mSampleTimeEntries = new (std::nothrow) SampleTimeEntry[mNumSampleSizes];
+ if (!mSampleTimeEntries)
+ return;

uint32_t sampleIndex = 0;
uint32_t sampleTime = 0;
--
1.9.1

From f5402b3b5c68d39eaa81a805fbe9e1ea65e86528 Mon Sep 17 00:00:00 2001
From: "Joshua J. Drake" <android-open-source@qoop.org>
Date: Wed, 8 Apr 2015 23:44:57 -0500
Subject: [PATCH 6/7] Fix integer overflow during MP4 atom processing

A few sample table related FourCC values are handled by the
setSampleToChunkParams function. An integer overflow exists within this
function. Validate that mNumSampleToChunkOffets will not cause an integer
overflow.

Change-Id: I4fc78c80d01ec4b7475e573a8e7d37ace4b5e399
---
media/libstagefright/SampleTable.cpp | 3 +++
1 file changed, 3 insertions(+)

diff --git a/media/libstagefright/SampleTable.cpp b/media/libstagefright/SampleTable.cpp
index bfae474..aba64d5 100644
--- a/media/libstagefright/SampleTable.cpp
+++ b/media/libstagefright/SampleTable.cpp
@@ -230,6 +230,9 @@ status_t SampleTable::setSampleToChunkParams(
return ERROR_MALFORMED;
}

+ if (SIZE_MAX / sizeof(SampleToChunkEntry) <= mNumSampleToChunkOffsets)
+ return ERROR_OUT_OF_RANGE;
+
mSampleToChunkEntries =
new (std::nothrow) SampleToChunkEntry[mNumSampleToChunkOffsets];
if (!mSampleToChunkEntries)
--
1.9.1

From c78f7e4894b676977090cb921820d05f80775dc5 Mon Sep 17 00:00:00 2001
From: "Joshua J. Drake" <android-open-source@qoop.org>
Date: Wed, 8 Apr 2015 23:53:10 -0500
Subject: [PATCH 7/7] Fix integer underflow in ESDS processing

Several arithmetic operations within parseESDescriptor could underflow, leading
to an out-of-bounds read operation. Ensure that subtractions from 'size' do not
cause it to wrap around.

Change-Id: Ie987c58e49323ff273fd57db410534fa83db1cb2
---
media/libstagefright/ESDS.cpp | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/media/libstagefright/ESDS.cpp b/media/libstagefright/ESDS.cpp
index 427bf7b..8fbb57c 100644
--- a/media/libstagefright/ESDS.cpp
+++ b/media/libstagefright/ESDS.cpp
@@ -136,6 +136,8 @@ status_t ESDS::parseESDescriptor(size_t offset, size_t size) {
--size;

if (streamDependenceFlag) {
+ if (size < 2)
+ return ERROR_MALFORMED;
offset += 2;
size -= 2;
}
@@ -145,11 +147,15 @@ status_t ESDS::parseESDescriptor(size_t offset, size_t size) {
return ERROR_MALFORMED;
}
unsigned URLlength = mData[offset];
+ if (URLlength >= size)
+ return ERROR_MALFORMED;
offset += URLlength + 1;
size -= URLlength + 1;
}

if (OCRstreamFlag) {
+ if (size < 2)
+ return ERROR_MALFORMED;
offset += 2;
size -= 2;

--
1.9.1

From a6d03717bc39c7773777009fa3f3896e3fa6c72e Mon Sep 17 00:00:00 2001
From: "Joshua J. Drake" <android-open-source@qoop.org>
Date: Mon, 4 May 2015 17:14:11 -0500
Subject: [PATCH 08/12] Fix integer underflow in covr MPEG4 processing

When the 'chunk_data_size' variable is less than 'kSkipBytesOfDataBox', an
integer underflow can occur. This causes an extraordinarily large value to
be passed to MetaData::setData, leading to a buffer overflow.

Change-Id: Icd28f63594ad941eabb3a12c750a4a2d5d2bf94b
---
media/libstagefright/MPEG4Extractor.cpp | 2 ++
1 file changed, 2 insertions(+)

diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp
index 44218ee..957bd6f 100644
--- a/media/libstagefright/MPEG4Extractor.cpp
+++ b/media/libstagefright/MPEG4Extractor.cpp
@@ -1989,6 +1989,8 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
return ERROR_IO;
}
const int kSkipBytesOfDataBox = 16;
+ if (chunk_data_size <= kSkipBytesOfDataBox)
+ return ERROR_MALFORMED;
mFileMetaData->setData(
kKeyAlbumArt, MetaData::TYPE_NONE,
buffer->data() + kSkipBytesOfDataBox, chunk_data_size - kSkipBytesOfDataBox);
--
1.9.1

From 33b7fce8352e78d27a7371b123ca6986f77290a2 Mon Sep 17 00:00:00 2001
From: "Joshua J. Drake" <android-open-source@qoop.org>
Date: Mon, 4 May 2015 17:33:49 -0500
Subject: [PATCH 09/12] Prevent reading past the end of the buffer in 3GPP

Metadata processed within the parse3GPPMetaData function may not be NUL
terminated and thus calling setCString may read out of bounds. Ensure
proper NUL termination, but take care not to interfere with other special
cases (ie, albm).

Change-Id: Ie93b3038b534b4c4460571a68f4d734cff7ad324
---
media/libstagefright/MPEG4Extractor.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp
index 957bd6f..6858e6d 100644
--- a/media/libstagefright/MPEG4Extractor.cpp
+++ b/media/libstagefright/MPEG4Extractor.cpp
@@ -2467,11 +2467,11 @@ status_t MPEG4Extractor::parseITunesMetaData(off64_t offset, size_t size) {
}

status_t MPEG4Extractor::parse3GPPMetaData(off64_t offset, size_t size, int depth) {
- if (size < 4) {
+ if (size < 4 || size == SIZE_MAX) {
return ERROR_MALFORMED;
}

- uint8_t *buffer = new (std::nothrow) uint8_t[size];
+ uint8_t *buffer = new (std::nothrow) uint8_t[size + 1];
if (buffer == NULL) {
return ERROR_MALFORMED;
}
@@ -2563,6 +2563,7 @@ status_t MPEG4Extractor::parse3GPPMetaData(off64_t offset, size_t size, int dept
}

if (isUTF8) {
+ buffer[size] = 0;
mFileMetaData->setCString(metadataKey, (const char *)buffer + 6);
} else {
// Convert from UTF-16 string to UTF-8 string.
--
1.9.1

From ab26d8fbda640f3a9e6d6bb8108a20514227f396 Mon Sep 17 00:00:00 2001
From: "Joshua J. Drake" <android-open-source@qoop.org>
Date: Mon, 4 May 2015 17:57:24 -0500
Subject: [PATCH 10/12] Prevent integer underflow if size is below 6

When processing 3GPP metadata, a subtraction operation may underflow and
lead to a rather large linear byteswap operation in the subsequent
framedata decoding code. Bound the 'size' value to prevent this from
occurring.

Change-Id: I35dfbc8878c6b65cfe8b8adb7351a77ad4d604e5
---
media/libstagefright/MPEG4Extractor.cpp | 3 +++
1 file changed, 3 insertions(+)

diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp
index 6858e6d..c689d6c 100644
--- a/media/libstagefright/MPEG4Extractor.cpp
+++ b/media/libstagefright/MPEG4Extractor.cpp
@@ -2540,6 +2540,9 @@ status_t MPEG4Extractor::parse3GPPMetaData(off64_t offset, size_t size, int dept
int len16 = 0; // Number of UTF-16 characters

// smallest possible valid UTF-16 string w BOM: 0xfe 0xff 0x00 0x00
+ if (size < 6)
+ return ERROR_MALFORMED;
+
if (size - 6 >= 4) {
len16 = ((size - 6) / 2) - 1; // don't include 0x0000 terminator
framedata = (char16_t *)(buffer + 6);
--
1.9.1

From 2bc6070da4c8cf39ba4408523e6c59e1c9a08bdc Mon Sep 17 00:00:00 2001
From: "Joshua J. Drake" <android-open-source@qoop.org>
Date: Mon, 4 May 2015 18:29:08 -0500
Subject: [PATCH 11/12] Fix integer overflow when handling MPEG4 tx3g atom

When the sum of the 'size' and 'chunk_size' variables is larger than 2^32,
an integer overflow occurs. Using the result value to allocate memory
leads to an undersized buffer allocation and later a potentially
exploitable heap corruption condition. Ensure that integer overflow does
not occur.

Change-Id: Id050a36b33196864bdd98b5ea24241f95a0b5d1f
---
media/libstagefright/MPEG4Extractor.cpp | 3 +++
1 file changed, 3 insertions(+)

diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp
index c689d6c..f01b543 100644
--- a/media/libstagefright/MPEG4Extractor.cpp
+++ b/media/libstagefright/MPEG4Extractor.cpp
@@ -1948,6 +1948,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
size = 0;
}

+ if (SIZE_MAX - chunk_size <= size)
+ return ERROR_MALFORMED;
+
uint8_t *buffer = new (std::nothrow) uint8_t[size + chunk_size];
if (buffer == NULL) {
return ERROR_MALFORMED;
--
1.9.1

From 5aa9b37a39b6231562700401533c3496aab55fc9 Mon Sep 17 00:00:00 2001
From: "Joshua J. Drake" <android-open-source@qoop.org>
Date: Mon, 4 May 2015 18:36:35 -0500
Subject: [PATCH 12/12] Prevent integer overflow when processing covr MPEG4
atoms

If the 'chunk_data_size' value is SIZE_MAX, an integer overflow will occur
and cause an undersized buffer to be allocated. The following processing
then overfills the resulting memory and creates a potentially exploitable
condition. Ensure that integer overflow does not occur.

Change-Id: I75cce323aec04a612e5a230ecd7c2077ce06035f
---
media/libstagefright/MPEG4Extractor.cpp | 3 +++
1 file changed, 3 insertions(+)

diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp
index f01b543..7a8521e 100644
--- a/media/libstagefright/MPEG4Extractor.cpp
+++ b/media/libstagefright/MPEG4Extractor.cpp
@@ -1986,6 +1986,9 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) {
if (mFileMetaData != NULL) {
ALOGV("chunk_data_size = %lld and data_offset = %lld",
chunk_data_size, data_offset);
+
+ if (chunk_data_size >= SIZE_MAX - 1)
+ return ERROR_MALFORMED;
sp<ABuffer> buffer = new ABuffer(chunk_data_size + 1);
if (mDataSource->readAt(
data_offset, buffer->data(), chunk_data_size) != (ssize_t)chunk_data_size) {
--
1.9.1