Fix security vulnerability: potential OOB write in audioserver am: e275907e57 am: 01e854056a am: 3e8ab60b7f am: 9161586309 am: ad29b47d91 am: 8b9b199891 am: 72729c449d am: 97bb7fe084 am: 3d6aada999 am: ffe82a3b4a am: ec601622f8 am: f50635bdc4 am: f1e829a54e am: a2e95f5c9b am: c6239b0d4d
am: cbc7183fbb
Change-Id: I7a13718ab24d1c4c9c64d83e148031af046921e1
diff --git a/media/libaudioclient/IEffect.cpp b/media/libaudioclient/IEffect.cpp
index 115ca75..ce72dae 100644
--- a/media/libaudioclient/IEffect.cpp
+++ b/media/libaudioclient/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 @@
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 @@
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);
diff --git a/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp b/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp
index a3d6761..af8cb50 100644
--- a/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp
+++ b/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp
@@ -3105,10 +3105,6 @@
//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 ||
@@ -3116,13 +3112,32 @@
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/services/audioflinger/Effects.cpp b/services/audioflinger/Effects.cpp
index f5e4f35..022bf59 100644
--- a/services/audioflinger/Effects.cpp
+++ b/services/audioflinger/Effects.cpp
@@ -612,6 +612,22 @@
android_errorWriteLog(0x534e4554, "29251553");
return -EINVAL;
}
+ if (cmdCode == EFFECT_CMD_GET_PARAM &&
+ (sizeof(effect_param_t) > *replySize
+ || ((effect_param_t *)pCmdData)->psize > *replySize
+ - sizeof(effect_param_t)
+ || ((effect_param_t *)pCmdData)->vsize > *replySize
+ - sizeof(effect_param_t)
+ - ((effect_param_t *)pCmdData)->psize
+ || roundUpDelta(((effect_param_t *)pCmdData)->psize, (uint32_t)sizeof(int)) >
+ *replySize
+ - sizeof(effect_param_t)
+ - ((effect_param_t *)pCmdData)->psize
+ - ((effect_param_t *)pCmdData)->vsize)) {
+ ALOGV("\tLVM_ERROR : EFFECT_CMD_GET_PARAM: reply size inconsistent");
+ android_errorWriteLog(0x534e4554, "32705438");
+ return -EINVAL;
+ }
if ((cmdCode == EFFECT_CMD_SET_PARAM
|| cmdCode == EFFECT_CMD_SET_PARAM_DEFERRED) && // DEFERRED not generally used
(sizeof(effect_param_t) > cmdSize