From 0574c56e88e96d33c923a8f54364ac0bf3dc5a91 Mon Sep 17 00:00:00 2001 From: rago Date: Tue, 22 Nov 2016 18:02:48 -0800 Subject: Fix security vulnerability: potential OOB write in audioserver Bug: 32705438 Bug: 32703959 Test: cts security test Change-Id: I8900c92fa55b56c4c2c9d721efdbabe6bfc8a4a4 (cherry picked from commit e275907e576601a3579747c3a842790bacf111e2) (cherry picked from commit b0bcddb44d992e74140a3f5eedc7177977ea8e34) --- .../libeffects/lvm/wrapper/Bundle/EffectBundle.cpp | 27 +++++++++++++++++----- media/libmedia/IEffect.cpp | 12 ++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) (limited to 'media') diff --git a/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp b/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp index 5e975b0..2588140 100644 --- a/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp +++ b/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp @@ -3124,10 +3124,6 @@ int Effect_command(effect_handle_t self, //ALOGV("\tEffect_command cmdCode Case: EFFECT_CMD_GET_PARAM start"); effect_param_t *p = (effect_param_t *)pCmdData; - if (SIZE_MAX - sizeof(effect_param_t) < (size_t)p->psize) { - android_errorWriteLog(0x534e4554, "26347509"); - return -EINVAL; - } if (pCmdData == NULL || cmdSize < sizeof(effect_param_t) || cmdSize < (sizeof(effect_param_t) + p->psize) || pReplyData == NULL || replySize == NULL || @@ -3135,13 +3131,32 @@ int Effect_command(effect_handle_t self, ALOGV("\tLVM_ERROR : EFFECT_CMD_GET_PARAM: ERROR"); return -EINVAL; } + if (EFFECT_PARAM_SIZE_MAX - sizeof(effect_param_t) < (size_t)p->psize) { + android_errorWriteLog(0x534e4554, "26347509"); + ALOGV("\tLVM_ERROR : EFFECT_CMD_GET_PARAM: psize too big"); + return -EINVAL; + } + uint32_t paddedParamSize = ((p->psize + sizeof(int32_t) - 1) / sizeof(int32_t)) * + sizeof(int32_t); + if ((EFFECT_PARAM_SIZE_MAX - sizeof(effect_param_t) < paddedParamSize) || + (EFFECT_PARAM_SIZE_MAX - sizeof(effect_param_t) - paddedParamSize < + p->vsize)) { + ALOGV("\tLVM_ERROR : EFFECT_CMD_GET_PARAM: padded_psize or vsize too big"); + return -EINVAL; + } + uint32_t expectedReplySize = sizeof(effect_param_t) + paddedParamSize + p->vsize; + if (*replySize < expectedReplySize) { + ALOGV("\tLVM_ERROR : EFFECT_CMD_GET_PARAM: min. replySize %u, got %u bytes", + expectedReplySize, *replySize); + android_errorWriteLog(0x534e4554, "32705438"); + return -EINVAL; + } memcpy(pReplyData, pCmdData, sizeof(effect_param_t) + p->psize); p = (effect_param_t *)pReplyData; - int voffset = ((p->psize - 1) / sizeof(int32_t) + 1) * sizeof(int32_t); - + uint32_t voffset = paddedParamSize; if(pContext->EffectType == LVM_BASS_BOOST){ p->status = android::BassBoost_getParameter(pContext, p->data, diff --git a/media/libmedia/IEffect.cpp b/media/libmedia/IEffect.cpp index faf5795..af6d8de 100644 --- a/media/libmedia/IEffect.cpp +++ b/media/libmedia/IEffect.cpp @@ -25,6 +25,9 @@ namespace android { +// Maximum command/reply size expected +#define EFFECT_PARAM_SIZE_MAX 65536 + enum { ENABLE = IBinder::FIRST_CALL_TRANSACTION, DISABLE, @@ -156,6 +159,10 @@ status_t BnEffect::onTransact( uint32_t cmdSize = data.readInt32(); char *cmd = NULL; if (cmdSize) { + if (cmdSize > EFFECT_PARAM_SIZE_MAX) { + reply->writeInt32(NO_MEMORY); + return NO_ERROR; + } cmd = (char *)calloc(cmdSize, 1); if (cmd == NULL) { reply->writeInt32(NO_MEMORY); @@ -167,6 +174,11 @@ status_t BnEffect::onTransact( uint32_t replySz = replySize; char *resp = NULL; if (replySize) { + if (replySize > EFFECT_PARAM_SIZE_MAX) { + free(cmd); + reply->writeInt32(NO_MEMORY); + return NO_ERROR; + } resp = (char *)calloc(replySize, 1); if (resp == NULL) { free(cmd); -- cgit v1.1 From 85473b3d8e9a5ed76a431924b21b0b10e19bc7a0 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Thu, 12 Jan 2017 15:49:04 -0800 Subject: Don't initialize sync sample parameters until the end to avoid leaving them in a partially initialized state. Bug: 33137046 Test: ran CTS tests CVE-2017-0483 Change-Id: I1f5c070233c5917d85da9e930e01a3fc51a0a0ec (cherry picked from commit a9660fe122ca382e1777e0c5d3c42ca67ffb0377) (cherry picked from commit bc62c086e9ba7530723dc8874b83159f4d77d976) --- media/libstagefright/SampleTable.cpp | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'media') diff --git a/media/libstagefright/SampleTable.cpp b/media/libstagefright/SampleTable.cpp index 8a38c24..2d7e613 100644 --- a/media/libstagefright/SampleTable.cpp +++ b/media/libstagefright/SampleTable.cpp @@ -512,8 +512,6 @@ status_t SampleTable::setSyncSampleParams(off64_t data_offset, size_t data_size) return ERROR_MALFORMED; } - mSyncSampleOffset = data_offset; - uint8_t header[8]; if (mDataSource->readAt( data_offset, header, sizeof(header)) < (ssize_t)sizeof(header)) { @@ -525,13 +523,13 @@ status_t SampleTable::setSyncSampleParams(off64_t data_offset, size_t data_size) return ERROR_MALFORMED; } - mNumSyncSamples = U32_AT(&header[4]); + uint32_t numSyncSamples = U32_AT(&header[4]); - if (mNumSyncSamples < 2) { + if (numSyncSamples < 2) { ALOGV("Table of sync samples is empty or has only a single entry!"); } - uint64_t allocSize = (uint64_t)mNumSyncSamples * sizeof(uint32_t); + uint64_t allocSize = (uint64_t)numSyncSamples * sizeof(uint32_t); if (allocSize > kMaxTotalSize) { ALOGE("Sync sample table size too large."); return ERROR_OUT_OF_RANGE; @@ -549,22 +547,27 @@ status_t SampleTable::setSyncSampleParams(off64_t data_offset, size_t data_size) return ERROR_OUT_OF_RANGE; } - mSyncSamples = new (std::nothrow) uint32_t[mNumSyncSamples]; + mSyncSamples = new (std::nothrow) uint32_t[numSyncSamples]; if (!mSyncSamples) { ALOGE("Cannot allocate sync sample table with %llu entries.", - (unsigned long long)mNumSyncSamples); + (unsigned long long)numSyncSamples); return ERROR_OUT_OF_RANGE; } - if (mDataSource->readAt(mSyncSampleOffset + 8, mSyncSamples, + if (mDataSource->readAt(data_offset + 8, mSyncSamples, (size_t)allocSize) != (ssize_t)allocSize) { + delete mSyncSamples; + mSyncSamples = NULL; return ERROR_IO; } - for (size_t i = 0; i < mNumSyncSamples; ++i) { + for (size_t i = 0; i < numSyncSamples; ++i) { mSyncSamples[i] = ntohl(mSyncSamples[i]) - 1; } + mSyncSampleOffset = data_offset; + mNumSyncSamples = numSyncSamples; + return OK; } -- cgit v1.1 From e9598179f3e286a58e8222a3654701b330cd5268 Mon Sep 17 00:00:00 2001 From: Robert Shih Date: Tue, 24 Jan 2017 18:08:59 -0800 Subject: avc_utils: skip empty NALs from malformed bistreams Avoid a CHECK and make it the decoder's repsonsibility to handle a malformed bistream gracefully. Bug: 34509901 Bug: 33137046 Test: StagefrightTest#testStagefright_bug_27855419_CVE_2016_2463 CVE-2017-0483 Change-Id: I2d94f8da63d65a86a9c711c45546e4c695e0f3b4 (cherry picked from commit 91fe76a157847825601b8f7a627efd1c9cbadcae) (cherry picked from commit 5cabe32a59f9be1e913b6a07a23d4cfa55e3fb2f) --- media/libstagefright/avc_utils.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'media') diff --git a/media/libstagefright/avc_utils.cpp b/media/libstagefright/avc_utils.cpp index 98b5c0e..bf014ba 100644 --- a/media/libstagefright/avc_utils.cpp +++ b/media/libstagefright/avc_utils.cpp @@ -454,7 +454,10 @@ bool IsAVCReferenceFrame(const sp &accessUnit) { size_t nalSize; bool bIsReferenceFrame = true; while (getNextNALUnit(&data, &size, &nalStart, &nalSize, true) == OK) { - CHECK_GT(nalSize, 0u); + if (nalSize == 0u) { + ALOGW("skipping empty nal unit from potentially malformed bitstream"); + continue; + } unsigned nalType = nalStart[0] & 0x1f; -- cgit v1.1 From f2c6b991081806343e038a687c8d8f63a747abd7 Mon Sep 17 00:00:00 2001 From: Jean-Michel Trivi Date: Mon, 13 Feb 2017 16:31:20 -0800 Subject: EffectBundle: check nb channels to write speaker angles When speaker angles are queried, the size of the array for the returned data is 3x the number of channels (where really it should be max(2, nbChannels)). The code assumed it was at least 3x2 (where 2 is the number of virtual speakers this effect supports) and would thus crash when called for a mono channel mask. Test: see repro steps in bug Bug: 32591350 AOSP-Change-Id: I33d4bff6b2e19a9fc4284a85a446804878d3a410 CVE-2017-0545 Change-Id: Ie4480d9abcfafcd53fca15ab2fd8ef7ecb6fd48d (cherry picked from commit e5a54485e08400a976092cd5b1c6d909d0e1a4ab) --- .../libeffects/lvm/wrapper/Bundle/EffectBundle.cpp | 24 ++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) (limited to 'media') diff --git a/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp b/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp index 2588140..9cddf6a 100644 --- a/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp +++ b/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp @@ -1465,17 +1465,25 @@ int VirtualizerForceVirtualizationMode(EffectContext *pContext, audio_devices_t // horizontal plane, +90 is directly above the user, -90 below // //---------------------------------------------------------------------------- -void VirtualizerGetSpeakerAngles(audio_channel_mask_t channelMask __unused, +void VirtualizerGetSpeakerAngles(audio_channel_mask_t channelMask, audio_devices_t deviceType __unused, int32_t *pSpeakerAngles) { // the channel count is guaranteed to be 1 or 2 // the device is guaranteed to be of type headphone - // this virtualizer is always 2in with speakers at -90 and 90deg of azimuth, 0deg of elevation - *pSpeakerAngles++ = (int32_t) AUDIO_CHANNEL_OUT_FRONT_LEFT; - *pSpeakerAngles++ = -90; // azimuth - *pSpeakerAngles++ = 0; // elevation - *pSpeakerAngles++ = (int32_t) AUDIO_CHANNEL_OUT_FRONT_RIGHT; - *pSpeakerAngles++ = 90; // azimuth - *pSpeakerAngles = 0; // elevation + // this virtualizer is always using 2 virtual speakers at -90 and 90deg of azimuth, 0deg of + // elevation but the return information is sized for nbChannels * 3, so we have to consider + // the (false here) case of a single channel, and return only 3 fields. + if (audio_channel_count_from_out_mask(channelMask) == 1) { + *pSpeakerAngles++ = (int32_t) AUDIO_CHANNEL_OUT_MONO; // same as FRONT_LEFT + *pSpeakerAngles++ = 0; // azimuth + *pSpeakerAngles = 0; // elevation + } else { + *pSpeakerAngles++ = (int32_t) AUDIO_CHANNEL_OUT_FRONT_LEFT; + *pSpeakerAngles++ = -90; // azimuth + *pSpeakerAngles++ = 0; // elevation + *pSpeakerAngles++ = (int32_t) AUDIO_CHANNEL_OUT_FRONT_RIGHT; + *pSpeakerAngles++ = 90; // azimuth + *pSpeakerAngles = 0; // elevation + } } //---------------------------------------------------------------------------- -- cgit v1.1 From 0836dbb870ff45470e595ec921fd5fab6e07d1ce Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Mon, 6 Feb 2017 14:12:30 -0800 Subject: Fix overflow check and check read result Bug: 33861560 Test: build AOSP-Change-Id: Ia85519766e19a6e37237166f309750b3e8323c4e CVE-2017-0547 (cherry picked from commit 9667e3eff2d34c3797c3b529370de47b2c1f1bf6) Change-Id: I171aa1c7c4a4a5095ac7041371db14e3a4f3676a --- media/libmedia/IHDCP.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'media') diff --git a/media/libmedia/IHDCP.cpp b/media/libmedia/IHDCP.cpp index f3a8902..e8c8a3d 100644 --- a/media/libmedia/IHDCP.cpp +++ b/media/libmedia/IHDCP.cpp @@ -241,14 +241,11 @@ status_t BnHDCP::onTransact( case HDCP_ENCRYPT: { size_t size = data.readInt32(); - size_t bufSize = 2 * size; - - // watch out for overflow void *inData = NULL; - if (bufSize > size) { - inData = malloc(bufSize); + // watch out for overflow + if (size <= SIZE_MAX / 2) { + inData = malloc(2 * size); } - if (inData == NULL) { reply->writeInt32(ERROR_OUT_OF_RANGE); return OK; @@ -256,11 +253,16 @@ status_t BnHDCP::onTransact( void *outData = (uint8_t *)inData + size; - data.read(inData, size); + status_t err = data.read(inData, size); + if (err != OK) { + free(inData); + reply->writeInt32(err); + return OK; + } uint32_t streamCTR = data.readInt32(); uint64_t inputCTR; - status_t err = encrypt(inData, size, streamCTR, &inputCTR, outData); + err = encrypt(inData, size, streamCTR, &inputCTR, outData); reply->writeInt32(err); -- cgit v1.1 From dc7805b0c79d056385a076422894425984af2aa0 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Mon, 13 Feb 2017 14:19:40 -0800 Subject: resolve merge conflicts of 79cf158c51 to mnc-dev AOSP-Change-Id: Ied32e83215e386c801c02991a0b2fa4baa25b643 CVE-2017-0558 (cherry picked from commit 50358a80b1724f6cf1bcdf003e1abf9cc141b122) Change-Id: Ic2e40c7d6aec8427444a1fd145726e490e994d08 --- media/libstagefright/wifi-display/rtp/RTPSender.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'media') diff --git a/media/libstagefright/wifi-display/rtp/RTPSender.cpp b/media/libstagefright/wifi-display/rtp/RTPSender.cpp index c66a898..83af393 100644 --- a/media/libstagefright/wifi-display/rtp/RTPSender.cpp +++ b/media/libstagefright/wifi-display/rtp/RTPSender.cpp @@ -762,10 +762,16 @@ status_t RTPSender::parseTSFB(const uint8_t *data, size_t size) { return OK; } -status_t RTPSender::parseAPP(const uint8_t *data, size_t size __unused) { - if (!memcmp("late", &data[8], 4)) { - int64_t avgLatencyUs = (int64_t)U64_AT(&data[12]); - int64_t maxLatencyUs = (int64_t)U64_AT(&data[20]); +status_t RTPSender::parseAPP(const uint8_t *data, size_t size) { + static const size_t late_offset = 8; + static const char late_string[] = "late"; + static const size_t avgLatencyUs_offset = late_offset + sizeof(late_string) - 1; + static const size_t maxLatencyUs_offset = avgLatencyUs_offset + sizeof(int64_t); + + if ((size >= (maxLatencyUs_offset + sizeof(int64_t))) + && !memcmp(late_string, &data[late_offset], sizeof(late_string) - 1)) { + int64_t avgLatencyUs = (int64_t)U64_AT(&data[avgLatencyUs_offset]); + int64_t maxLatencyUs = (int64_t)U64_AT(&data[maxLatencyUs_offset]); sp notify = mNotify->dup(); notify->setInt32("what", kWhatInformSender); -- cgit v1.1