DO NOT MERGE HFP: Check AT command buffer boundary during parsing

* add p_end parameter to tBTA_AG_AT_CMD_CBACK, bta_ag_at_hsp_cback
  and bta_ag_at_hfp_cback to indicate effective data range of p_arg
* add checks for buffer copy overflow in bta_ag_at_hsp_cback and
  bta_ag_at_hfp_cback
* add packet legnth checks with p_end in bta_ag_parse_cmer
* add packet length checks with p_end in bta_ag_parse_bac

Bug: 112860487
Test: manual
Change-Id: Idbfa2b8bd4c1a0aeeacfe34349851b3bc8de7c69
(cherry picked from commit 5b1ef1038e3f4e4371c3d6718bf0f684be65eb2b)
(cherry picked from commit aea10aec7f7e97e9c02f57adf455bdba9e13f210)

diff --git a/bta/ag/bta_ag_act.c b/bta/ag/bta_ag_act.c
index 285a967..e93acb2 100644 (file)
--- a/bta/ag/bta_ag_act.c
+++ b/bta/ag/bta_ag_act.c
@@ -68,7 +68,7 @@ const tBTA_SERVICE_MASK bta_ag_svc_mask[BTA_AG_NUM_IDX] =
 };
 
 typedef void (*tBTA_AG_ATCMD_CBACK)(tBTA_AG_SCB *p_scb, UINT16 cmd, UINT8 arg_type,
-                                    char *p_arg, INT16 int_arg);
+                                    char *p_arg, char *p_end, INT16 int_arg);
 
 const tBTA_AG_ATCMD_CBACK bta_ag_at_cback_tbl[BTA_AG_NUM_IDX] =
 {

diff --git a/bta/ag/bta_ag_at.c b/bta/ag/bta_ag_at.c
index 3606175..952f1f8 100644 (file)
--- a/bta/ag/bta_ag_at.c
+++ b/bta/ag/bta_ag_at.c
@@ -25,6 +25,7 @@
 #include <string.h>
 #include "bt_common.h"
 #include "bta_ag_at.h"
+#include "log/log.h"
 #include "utl.h"
 
 /*****************************************************************************
@@ -77,7 +78,7 @@ void bta_ag_at_reinit(tBTA_AG_AT_CB *p_cb)
 ** Returns          void
 **
 ******************************************************************************/
-void bta_ag_process_at(tBTA_AG_AT_CB *p_cb)
+void bta_ag_process_at(tBTA_AG_AT_CB *p_cb, char *p_end)
 {
     UINT16      idx;
     UINT8       arg_type;
@@ -97,6 +98,11 @@ void bta_ag_process_at(tBTA_AG_AT_CB *p_cb)
     {
         /* start of argument is p + strlen matching command */
         p_arg = p_cb->p_cmd_buf + strlen(p_cb->p_at_tbl[idx].p_cmd);
+        if (p_arg > p_end) {
+            (*p_cb->p_err_cback)(p_cb->p_user, FALSE, NULL);
+            android_errorWriteLog(0x534e4554, "112860487");
+            return;
+        }
 
         /* if no argument */
         if (p_arg[0] == 0)
@@ -151,14 +157,14 @@ void bta_ag_process_at(tBTA_AG_AT_CB *p_cb)
 
                     (*p_cb->p_cmd_cback)(p_cb->p_user,
                                          p_cb->p_at_tbl[idx].command_id,
-                                         arg_type, p_arg, int_arg);
+                                         arg_type, p_arg, p_end, int_arg);
                 }
             }
             else
             {
                 (*p_cb->p_cmd_cback)(p_cb->p_user,
                                      p_cb->p_at_tbl[idx].command_id,
-                                     arg_type, p_arg, int_arg);
+                                     arg_type, p_arg, p_end, int_arg);
             }
         }
         /* else error */
@@ -216,8 +222,9 @@ void bta_ag_at_parse(tBTA_AG_AT_CB *p_cb, char *p_buf, UINT16 len)
                     (p_cb->p_cmd_buf[1] == 'T' || p_cb->p_cmd_buf[1] == 't'))
                 {
                     p_save = p_cb->p_cmd_buf;
+                    char *p_end = p_cb->p_cmd_buf + p_cb->cmd_pos;
                     p_cb->p_cmd_buf += 2;
-                    bta_ag_process_at(p_cb);
+                    bta_ag_process_at(p_cb, p_end);
                     p_cb->p_cmd_buf = p_save;
                 }
 

diff --git a/bta/ag/bta_ag_at.h b/bta/ag/bta_ag_at.h
index 4b78a33..b2dae6e 100644 (file)
--- a/bta/ag/bta_ag_at.h
+++ b/bta/ag/bta_ag_at.h
@@ -56,7 +56,7 @@ typedef struct
 
 /* callback function executed when command is parsed */
 typedef void (tBTA_AG_AT_CMD_CBACK)(void *p_user, UINT16 command_id, UINT8 arg_type,
-                                    char *p_arg, INT16 int_arg);
+                                    char *p_arg, char *p_end, INT16 int_arg);
 
 /* callback function executed to send "ERROR" result code */
 typedef void (tBTA_AG_AT_ERR_CBACK)(void *p_user, BOOLEAN unknown, char *p_arg);

diff --git a/bta/ag/bta_ag_cmd.c b/bta/ag/bta_ag_cmd.c
index ebc4f4f..7a882ec 100644 (file)
--- a/bta/ag/bta_ag_cmd.c
+++ b/bta/ag/bta_ag_cmd.c
@@ -29,6 +29,7 @@
 #include "bta_ag_int.h"
 #include "bta_api.h"
 #include "bta_sys.h"
+#include "log/log.h"
 #include "bt_common.h"
 #include "osi/include/log.h"
 #include "port_api.h"
@@ -412,25 +413,25 @@ static void bta_ag_send_ind(tBTA_AG_SCB *p_scb, UINT16 id, UINT16 value, BOOLEAN
 ** Returns          TRUE if parsed ok, FALSE otherwise.
 **
 *******************************************************************************/
-static BOOLEAN bta_ag_parse_cmer(char *p_s, BOOLEAN *p_enabled)
+static BOOLEAN bta_ag_parse_cmer(char *p_s, char *p_end, BOOLEAN *p_enabled)
 {
     INT16   n[4] = {-1, -1, -1, -1};
     int     i;
     char    *p;
 
-    for (i = 0; i < 4; i++)
+    for (i = 0; i < 4; i++, p_s = p + 1)
     {
         /* skip to comma delimiter */
-        for (p = p_s; *p != ',' && *p != 0; p++);
+        for (p = p_s; p < p_end && *p != ',' && *p != 0; p++);
 
         /* get integer value */
+        if (p > p_end) {
+            android_errorWriteLog(0x534e4554, "112860487");
+            return FALSE;
+        }
         *p = 0;
         n[i] = utl_str2int(p_s);
         p_s = p + 1;
-        if (p_s == 0)
-        {
-            break;
-        }
     }
 
     /* process values */
@@ -496,7 +497,8 @@ static UINT8 bta_ag_parse_chld(tBTA_AG_SCB *p_scb, char *p_s)
 ** Returns          Returns bitmap of supported codecs.
 **
 *******************************************************************************/
-static tBTA_AG_PEER_CODEC bta_ag_parse_bac(tBTA_AG_SCB *p_scb, char *p_s)
+static tBTA_AG_PEER_CODEC bta_ag_parse_bac(tBTA_AG_SCB *p_scb, char *p_s,
+                                           char *p_end)
 {
     tBTA_AG_PEER_CODEC  retval = BTA_AG_CODEC_NONE;
     UINT16  uuid_codec;
@@ -506,9 +508,13 @@ static tBTA_AG_PEER_CODEC bta_ag_parse_bac(tBTA_AG_SCB *p_scb, char *p_s)
     while(p_s)
     {
         /* skip to comma delimiter */
-        for(p = p_s; *p != ',' && *p != 0; p++);
+        for(p = p_s; p < p_end && *p != ',' && *p != 0; p++);
 
         /* get integre value */
+        if (p > p_end) {
+            android_errorWriteLog(0x534e4554, "112860487");
+            break;
+        }
         if (*p != 0)
         {
             *p = 0;
@@ -660,7 +666,7 @@ void bta_ag_send_call_inds(tBTA_AG_SCB *p_scb, tBTA_AG_RES result)
 **
 *******************************************************************************/
 void bta_ag_at_hsp_cback(tBTA_AG_SCB *p_scb, UINT16 command_id, UINT8 arg_type,
-                                char *p_arg, INT16 int_arg)
+                                char *p_arg, char *p_end, INT16 int_arg)
 {
     APPL_TRACE_DEBUG("AT cmd:%d arg_type:%d arg:%d arg:%s", command_id, arg_type,
                       int_arg, p_arg);
@@ -671,7 +677,14 @@ void bta_ag_at_hsp_cback(tBTA_AG_SCB *p_scb, UINT16 command_id, UINT8 arg_type,
     val.hdr.handle = bta_ag_scb_to_idx(p_scb);
     val.hdr.app_id = p_scb->app_id;
     val.num = (UINT16) int_arg;
-    strlcpy(val.str, p_arg, BTA_AG_AT_MAX_LEN);
+
+    if ((p_end - p_arg + 1) >= (long)sizeof(val.str)) {
+        APPL_TRACE_ERROR("%s: p_arg is too long, send error and return", __func__);
+        bta_ag_send_error(p_scb, BTA_AG_ERR_TEXT_TOO_LONG);
+        android_errorWriteLog(0x534e4554, "112860487");
+        return;
+    }
+    strlcpy(val.str, p_arg, sizeof(val.str));
 
     /* call callback with event */
     (*bta_ag_cb.p_cback)(command_id, (tBTA_AG *) &val);
@@ -904,7 +917,7 @@ static bool bta_ag_parse_biev_response(tBTA_AG_SCB *p_scb, tBTA_AG_VAL *val)
 **
 *******************************************************************************/
 void bta_ag_at_hfp_cback(tBTA_AG_SCB *p_scb, UINT16 cmd, UINT8 arg_type,
-                                char *p_arg, INT16 int_arg)
+                                char *p_arg, char *p_end, INT16 int_arg)
 {
     tBTA_AG_VAL     val;
     tBTA_AG_SCB     *ag_scb;
@@ -929,7 +942,14 @@ void bta_ag_at_hfp_cback(tBTA_AG_SCB *p_scb, UINT16 cmd, UINT8 arg_type,
     val.hdr.status = BTA_AG_SUCCESS;
     val.num = int_arg;
     bdcpy(val.bd_addr, p_scb->peer_addr);
-    strlcpy(val.str, p_arg, BTA_AG_AT_MAX_LEN);
+
+    if ((p_end - p_arg + 1) >= (long)sizeof(val.str)) {
+        APPL_TRACE_ERROR("%s: p_arg is too long, send error and return", __func__);
+        bta_ag_send_error(p_scb, BTA_AG_ERR_TEXT_TOO_LONG);
+        android_errorWriteLog(0x534e4554, "112860487");
+        return;
+    }
+    strlcpy(val.str, p_arg, sizeof(val.str));
 
     /**
      * Unless this this is a local event, by default we'll forward
@@ -1115,7 +1135,7 @@ void bta_ag_at_hfp_cback(tBTA_AG_SCB *p_scb, UINT16 cmd, UINT8 arg_type,
 
         case BTA_AG_LOCAL_EVT_CMER:
             /* if parsed ok store setting, send OK */
-            if (bta_ag_parse_cmer(p_arg, &p_scb->cmer_enabled))
+            if (bta_ag_parse_cmer(p_arg, p_end, &p_scb->cmer_enabled))
             {
                 bta_ag_send_ok(p_scb);
 
@@ -1304,7 +1324,7 @@ void bta_ag_at_hfp_cback(tBTA_AG_SCB *p_scb, UINT16 cmd, UINT8 arg_type,
             /* store available codecs from the peer */
             if((p_scb->peer_features & BTA_AG_PEER_FEAT_CODEC) && (p_scb->features & BTA_AG_FEAT_CODEC))
             {
-                p_scb->peer_codecs = bta_ag_parse_bac(p_scb, p_arg);
+                p_scb->peer_codecs = bta_ag_parse_bac(p_scb, p_arg, p_end);
                 p_scb->codec_updated = TRUE;
 
                 if (p_scb->peer_codecs & BTA_AG_CODEC_MSBC)

diff --git a/bta/ag/bta_ag_int.h b/bta/ag/bta_ag_int.h
index c1e685f..eac98c2 100644 (file)
--- a/bta/ag/bta_ag_int.h
+++ b/bta/ag/bta_ag_int.h
@@ -378,9 +378,9 @@ extern void bta_ag_sco_conn_rsp(tBTA_AG_SCB *p_scb, tBTM_ESCO_CONN_REQ_EVT_DATA
 
 /* AT command functions */
 extern void bta_ag_at_hsp_cback(tBTA_AG_SCB *p_scb, UINT16 cmd, UINT8 arg_type,
-                                char *p_arg, INT16 int_arg);
+                                char *p_arg, char *p_end, INT16 int_arg);
 extern void bta_ag_at_hfp_cback(tBTA_AG_SCB *p_scb, UINT16 cmd, UINT8 arg_type,
-                                char *p_arg, INT16 int_arg);
+                                char *p_arg, char *p_end, INT16 int_arg);
 extern void bta_ag_at_err_cback(tBTA_AG_SCB *p_scb, BOOLEAN unknown, char *p_arg);
 extern BOOLEAN bta_ag_inband_enabled(tBTA_AG_SCB *p_scb);
 extern void bta_ag_send_call_inds(tBTA_AG_SCB *p_scb, tBTA_AG_RES result);