From 0574c56e88e96d33c923a8f54364ac0bf3dc5a91 Mon Sep 17 00:00:00 2001
From: rago <rago@google.com>
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