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 +++++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) (limited to 'media/libeffects/lvm') 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, -- 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/libeffects/lvm') 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