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: testplans/details/218593/3975
Change-Id: I6bbbc2ba29ad025c7d3ba023d8191af6a11c4aa9
(cherry picked from commit 28ddbe904bd15c9636063f5431a9360d8e9df8b9)
diff --git a/bta/ag/bta_ag_act.cc b/bta/ag/bta_ag_act.cc
index 84860f9..eb989eb 100644
--- a/bta/ag/bta_ag_act.cc
+++ b/bta/ag/bta_ag_act.cc
@@ -58,7 +58,7 @@
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 ecd9053..90c46ff 100644
--- 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 @@
* 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 @@
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)((tBTA_AG_SCB*)p_cb->p_user, false, nullptr);
+ android_errorWriteLog(0x534e4554, "112860487");
+ return;
+ }
/* if no argument */
if (p_arg[0] == 0) {
@@ -133,12 +139,12 @@
} else {
(*p_cb->p_cmd_cback)((tBTA_AG_SCB*)p_cb->p_user,
p_cb->p_at_tbl[idx].command_id, arg_type, p_arg,
- int_arg);
+ p_end, int_arg);
}
} else {
(*p_cb->p_cmd_cback)((tBTA_AG_SCB*)p_cb->p_user,
p_cb->p_at_tbl[idx].command_id, arg_type, p_arg,
- int_arg);
+ p_end, int_arg);
}
}
/* else error */
@@ -189,8 +195,9 @@
(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 f36e2f2..c5b5d88 100644
--- a/bta/ag/bta_ag_at.h
+++ b/bta/ag/bta_ag_at.h
@@ -56,7 +56,7 @@
/* callback function executed when command is parsed */
struct tBTA_AG_SCB;
typedef void(tBTA_AG_AT_CMD_CBACK)(tBTA_AG_SCB* 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 b9cf306..2489915 100644
--- 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"
@@ -376,23 +377,23 @@
* 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 == nullptr) {
- break;
- }
}
/* process values */
@@ -449,17 +450,22 @@
* 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;
char* p;
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 integer value */
+ if (p > p_end) {
+ android_errorWriteLog(0x534e4554, "112860487");
+ break;
+ }
bool cont = false; // Continue processing
if (*p != 0) {
*p = 0;
@@ -584,7 +590,8 @@
*
******************************************************************************/
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);
@@ -594,6 +601,13 @@
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 */
@@ -824,7 +838,7 @@
*
******************************************************************************/
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;
@@ -843,6 +857,13 @@
val.hdr.status = BTA_AG_SUCCESS;
val.num = static_cast<uint32_t>(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));
/**
@@ -1023,7 +1044,7 @@
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
@@ -1191,7 +1212,7 @@
/* 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 c6b3230..6393450 100644
--- a/bta/ag/bta_ag_int.h
+++ b/bta/ag/bta_ag_int.h
@@ -344,9 +344,11 @@
/* 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,
const char* p_arg);
extern bool bta_ag_inband_enabled(tBTA_AG_SCB* p_scb);
