audio effects: fix heap overflow

Check consistency of effect command reply sizes before
copying to reply address.

Also add null pointer check on reply size.
Also remove unused parameter warning.

Bug: 21953516.
Change-Id: I4cf00c12eaed696af28f3b7613f7e36f47a160c4

Signed-off-by: Eric Laurent <elaurent@google.com>
Tested-by: Moritz Bandemer <replicant@posteo.mx>

1 files changed, 19 insertions, 16 deletions

diff --git a/media/libeffects/preprocessing/PreProcessing.cpp b/media/libeffects/preprocessing/PreProcessing.cpp
index 597866a..fe9acf6 100755
--- a/media/libeffects/preprocessing/PreProcessing.cpp
+++ b/media/libeffects/preprocessing/PreProcessing.cpp
@@ -575,16 +575,18 @@ int  NsCreate(preproc_effect_t *effect)
     return 0;
 }
 
-int NsGetParameter(preproc_effect_t     *effect,
-                   void              *pParam,
-                   size_t            *pValueSize,
-                   void              *pValue)
+int NsGetParameter(preproc_effect_t  *effect __unused,
+                   void              *pParam __unused,
+                   uint32_t          *pValueSize __unused,
+                   void              *pValue __unused)
 {
     int status = 0;
     return status;
 }
 
-int NsSetParameter (preproc_effect_t *effect, void *pParam, void *pValue)
+int NsSetParameter (preproc_effect_t *effect __unused,
+                    void *pParam __unused,
+                    void *pValue __unused)
 {
     int status = 0;
     return status;
@@ -1434,16 +1436,17 @@ int PreProcessingFx_Command(effect_handle_t  self,
             }
             break;
 
-        case EFFECT_CMD_GET_PARAM:{
-            if (pCmdData == NULL ||
-                    cmdSize < (int)sizeof(effect_param_t) ||
-                    pReplyData == NULL ||
-                    *replySize < (int)sizeof(effect_param_t)){
+        case EFFECT_CMD_GET_PARAM: {
+            effect_param_t *p = (effect_param_t *)pCmdData;
+
+            if (pCmdData == NULL || cmdSize < sizeof(effect_param_t) ||
+                    cmdSize < (sizeof(effect_param_t) + p->psize) ||
+                    pReplyData == NULL || replySize == NULL ||
+                    *replySize < (sizeof(effect_param_t) + p->psize)){
                 ALOGV("PreProcessingFx_Command cmdCode Case: "
                         "EFFECT_CMD_GET_PARAM: ERROR");
                 return -EINVAL;
             }
-            effect_param_t *p = (effect_param_t *)pCmdData;
 
             memcpy(pReplyData, pCmdData, sizeof(effect_param_t) + p->psize);
 
@@ -1461,8 +1464,8 @@ int PreProcessingFx_Command(effect_handle_t  self,
 
         case EFFECT_CMD_SET_PARAM:{
             if (pCmdData == NULL||
-                    cmdSize < (int)sizeof(effect_param_t) ||
-                    pReplyData == NULL ||
+                    cmdSize < sizeof(effect_param_t) ||
+                    pReplyData == NULL || replySize == NULL ||
                     *replySize != sizeof(int32_t)){
                 ALOGV("PreProcessingFx_Command cmdCode Case: "
                         "EFFECT_CMD_SET_PARAM: ERROR");
@@ -1483,7 +1486,7 @@ int PreProcessingFx_Command(effect_handle_t  self,
         } break;
 
         case EFFECT_CMD_ENABLE:
-            if (pReplyData == NULL || *replySize != sizeof(int)){
+            if (pReplyData == NULL || replySize == NULL || *replySize != sizeof(int)){
                 ALOGV("PreProcessingFx_Command cmdCode Case: EFFECT_CMD_ENABLE: ERROR");
                 return -EINVAL;
             }
@@ -1491,7 +1494,7 @@ int PreProcessingFx_Command(effect_handle_t  self,
             break;
 
         case EFFECT_CMD_DISABLE:
-            if (pReplyData == NULL || *replySize != sizeof(int)){
+            if (pReplyData == NULL || replySize == NULL || *replySize != sizeof(int)){
                 ALOGV("PreProcessingFx_Command cmdCode Case: EFFECT_CMD_DISABLE: ERROR");
                 return -EINVAL;
             }
@@ -1711,7 +1714,7 @@ int PreProcessingFx_GetDescriptor(effect_handle_t   self,
 
 int PreProcessingFx_ProcessReverse(effect_handle_t     self,
                                    audio_buffer_t    *inBuffer,
-                                   audio_buffer_t    *outBuffer)
+                                   audio_buffer_t    *outBuffer __unused)
 {
     preproc_effect_t * effect = (preproc_effect_t *)self;
     int    status = 0;