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: I6bbbc2ba29ad025c7d3ba023d8191af6a11c4aa9
(cherry picked from commit 749063afebb8324276a47bdfbf320aa70f94a8ba)
(cherry picked from commit 9cb959d00d33737b399377cfc0f4070081d48f5e)

diff --git a/bta/ag/bta_ag_act.cc b/bta/ag/bta_ag_act.cc
index ad40c5d..c583054 100644 (file)
--- a/bta/ag/bta_ag_act.cc
+++ b/bta/ag/bta_ag_act.cc
@@ -58,7 +58,7 @@ const tBTA_SERVICE_MASK bta_ag_svc_mask[BTA_AG_NUM_IDX] = {
     BTA_HSP_SERVICE_MASK, BTA_HFP_SERVICE_MASK};
 
 typedef void (*tBTA_AG_ATCMD_CBACK)(tBTA_AG_SCB* p_scb, uint16_t cmd,
-                                    uint8_t arg_type, char* p_arg,
+                                    uint8_t arg_type, char* p_arg, char* p_end,
                                     int16_t int_arg);
 
 const tBTA_AG_ATCMD_CBACK bta_ag_at_cback_tbl[BTA_AG_NUM_IDX] = {

diff --git a/bta/ag/bta_ag_at.cc b/bta/ag/bta_ag_at.cc
index 3e4868b..7fde8e0 100644 (file)
--- a/bta/ag/bta_ag_at.cc
+++ b/bta/ag/bta_ag_at.cc
@@ -26,6 +26,7 @@
 
 #include "bt_common.h"
 #include "bta_ag_at.h"
+#include "log/log.h"
 #include "utl.h"
 
 /*****************************************************************************
@@ -76,7 +77,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_t idx;
   uint8_t arg_type;
   char* p_arg;
@@ -92,6 +93,11 @@ void bta_ag_process_at(tBTA_AG_AT_CB* p_cb) {
   if (p_cb->p_at_tbl[idx].p_cmd[0] != 0) {
     /* 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) {
@@ -132,11 +138,11 @@ void bta_ag_process_at(tBTA_AG_AT_CB* p_cb) {
           (*p_cb->p_err_cback)(p_cb->p_user, false, NULL);
         } 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 {
         (*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 */
@@ -187,8 +193,9 @@ void bta_ag_at_parse(tBTA_AG_AT_CB* p_cb, char* p_buf, uint16_t len) {
             (p_cb->p_cmd_buf[0] == 'A' || p_cb->p_cmd_buf[0] == 'a') &&
             (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 144bc38..71ac442 100644 (file)
--- a/bta/ag/bta_ag_at.h
+++ b/bta/ag/bta_ag_at.h
@@ -55,7 +55,7 @@ typedef struct {
 
 /* callback function executed when command is parsed */
 typedef void(tBTA_AG_AT_CMD_CBACK)(void* p_user, uint16_t command_id,
-                                   uint8_t arg_type, char* p_arg,
+                                   uint8_t arg_type, char* p_arg, char* p_end,
                                    int16_t int_arg);
 
 /* callback function executed to send "ERROR" result code */

diff --git a/bta/ag/bta_ag_cmd.cc b/bta/ag/bta_ag_cmd.cc
index 79028db..dbd38ed 100644 (file)
--- a/bta/ag/bta_ag_cmd.cc
+++ b/bta/ag/bta_ag_cmd.cc
@@ -30,6 +30,7 @@
 #include "bta_ag_int.h"
 #include "bta_api.h"
 #include "bta_sys.h"
+#include "log/log.h"
 #include "osi/include/log.h"
 #include "osi/include/osi.h"
 #include "port_api.h"
@@ -378,23 +379,23 @@ static void bta_ag_send_ind(tBTA_AG_SCB* p_scb, uint16_t id, uint16_t value,
  * Returns          true if parsed ok, false otherwise.
  *
  ******************************************************************************/
-static bool bta_ag_parse_cmer(char* p_s, bool* p_enabled) {
+static bool bta_ag_parse_cmer(char* p_s, char* p_end, bool* p_enabled) {
   int16_t 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 */
@@ -452,7 +453,8 @@ static uint8_t bta_ag_parse_chld(UNUSED_ATTR 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_t uuid_codec;
   bool cont = false; /* Continue processing */
@@ -460,10 +462,14 @@ 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;
       cont = true;
@@ -597,7 +603,8 @@ 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_t command_id,
-                         uint8_t arg_type, char* p_arg, int16_t int_arg) {
+                         uint8_t arg_type, char* p_arg, char* p_end,
+                         int16_t int_arg) {
   APPL_TRACE_DEBUG("AT cmd:%d arg_type:%d arg:%d arg:%s", command_id, arg_type,
                    int_arg, p_arg);
 
@@ -607,6 +614,13 @@ void bta_ag_at_hsp_cback(tBTA_AG_SCB* p_scb, uint16_t command_id,
   val.hdr.handle = bta_ag_scb_to_idx(p_scb);
   val.hdr.app_id = p_scb->app_id;
   val.num = (uint16_t)int_arg;
+
+  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 */
@@ -836,7 +850,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_t cmd, uint8_t arg_type,
-                         char* p_arg, int16_t int_arg) {
+                         char* p_arg, char* p_end, int16_t int_arg) {
   tBTA_AG_VAL val;
   tBTA_AG_SCB* ag_scb;
   uint32_t i, ind_id;
@@ -856,6 +870,13 @@ void bta_ag_at_hfp_cback(tBTA_AG_SCB* p_scb, uint16_t cmd, uint8_t arg_type,
   val.hdr.status = BTA_AG_SUCCESS;
   val.num = int_arg;
   val.bd_addr = p_scb->peer_addr;
+
+  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));
 
   /**
@@ -1034,7 +1055,7 @@ void bta_ag_at_hfp_cback(tBTA_AG_SCB* p_scb, uint16_t cmd, uint8_t 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);
 
         /* if service level conn. not already open and our features and
@@ -1195,7 +1216,7 @@ void bta_ag_at_hfp_cback(tBTA_AG_SCB* p_scb, uint16_t cmd, uint8_t 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 3ebf369..bb40f22 100644 (file)
--- a/bta/ag/bta_ag_int.h
+++ b/bta/ag/bta_ag_int.h
@@ -361,9 +361,11 @@ extern void bta_ag_sco_conn_rsp(tBTA_AG_SCB* p_scb,
 
 /* AT command functions */
 extern void bta_ag_at_hsp_cback(tBTA_AG_SCB* p_scb, uint16_t cmd,
-                                uint8_t arg_type, char* p_arg, int16_t int_arg);
+                                uint8_t arg_type, char* p_arg, char* p_end,
+                                int16_t int_arg);
 extern void bta_ag_at_hfp_cback(tBTA_AG_SCB* p_scb, uint16_t cmd,
-                                uint8_t arg_type, char* p_arg, int16_t int_arg);
+                                uint8_t arg_type, char* p_arg, char* p_end,
+                                int16_t int_arg);
 extern void bta_ag_at_err_cback(tBTA_AG_SCB* p_scb, bool unknown, char* p_arg);
 extern bool 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);