diff options
author | Chienyuan <chienyuanhuang@google.com> | 2018-09-18 17:13:16 +0800 |
---|---|---|
committer | Tim Schumacher <timschumi@gmx.de> | 2019-02-10 12:18:19 +0100 |
commit | 108912d72017f3273081c1106acd539bf8be7a6c (patch) | |
tree | 3b167dab8800f9e7c162793d28a032ecd6f6e949 | |
parent | 4258d0b4170bc75d0658911aa805e032b9642ef1 (diff) | |
download | android_system_bt-108912d72017f3273081c1106acd539bf8be7a6c.tar.gz android_system_bt-108912d72017f3273081c1106acd539bf8be7a6c.tar.bz2 android_system_bt-108912d72017f3273081c1106acd539bf8be7a6c.zip |
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)
CVE-2018-9583
-rw-r--r-- | bta/ag/bta_ag_act.c | 2 | ||||
-rw-r--r-- | bta/ag/bta_ag_at.c | 19 | ||||
-rw-r--r-- | bta/ag/bta_ag_at.h | 2 | ||||
-rw-r--r-- | bta/ag/bta_ag_cmd.c | 49 | ||||
-rw-r--r-- | bta/ag/bta_ag_int.h | 4 |
5 files changed, 54 insertions, 22 deletions
diff --git a/bta/ag/bta_ag_act.c b/bta/ag/bta_ag_act.c index f66bdff53..d4eaf6fa7 100644 --- a/bta/ag/bta_ag_act.c +++ b/bta/ag/bta_ag_act.c @@ -71,7 +71,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 66480401a..0bedc45a6 100644 --- a/bta/ag/bta_ag_at.c +++ b/bta/ag/bta_ag_at.c @@ -25,6 +25,8 @@ #include <string.h> #include "gki.h" #include "bta_ag_at.h" +#include "bta_ag_int.h" +#include "log/log.h" #include "utl.h" /***************************************************************************** @@ -80,7 +82,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; @@ -100,6 +102,12 @@ 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)((tBTA_AG_SCB*)p_cb->p_user, false, NULL); + android_errorWriteLog(0x534e4554, "112860487"); + return; + } /* if no argument */ if (p_arg[0] == 0) @@ -152,12 +160,14 @@ void bta_ag_process_at(tBTA_AG_AT_CB *p_cb) else { - (*p_cb->p_cmd_cback)(p_cb->p_user, idx, arg_type, p_arg, int_arg); + (*p_cb->p_cmd_cback)(p_cb->p_user, idx, arg_type, p_arg, + p_end, int_arg); } } else { - (*p_cb->p_cmd_cback)(p_cb->p_user, idx, arg_type, p_arg, int_arg); + (*p_cb->p_cmd_cback)(p_cb->p_user, idx, arg_type, p_arg, + p_end, int_arg); } } /* else error */ @@ -220,8 +230,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 90d7b0fd2..c0ba7fb7f 100644 --- 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 cmd, 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 a5c34bc0a..f45384f87 100644 --- a/bta/ag/bta_ag_cmd.c +++ b/bta/ag/bta_ag_cmd.c @@ -34,6 +34,7 @@ #include "bta_api.h" #include "bta_sys.h" #include "gki.h" +#include "log/log.h" #include "port_api.h" #include "utl.h" @@ -602,25 +603,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 */ - *p = 0; - n[i] = utl_str2int(p_s); - p_s = p + 1; - if (p_s == 0) + if (p > p_end) { - break; + android_errorWriteLog(0x534e4554, "112860487"); + return false; } + *p = 0; + n[i] = utl_str2int(p_s); } /* process values */ @@ -748,7 +749,8 @@ static BOOLEAN bta_ag_parse_biev(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; @@ -758,9 +760,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; @@ -968,7 +975,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 cmd, UINT8 arg_type, - char *p_arg, INT16 int_arg) + char *p_arg, char *p_end, INT16 int_arg) { tBTA_AG_VAL val; @@ -981,6 +988,13 @@ void bta_ag_at_hsp_cback(tBTA_AG_SCB *p_scb, UINT16 cmd, 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; + + 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; + } BCM_STRNCPY_S(val.str, sizeof(val.str), p_arg, BTA_AG_AT_MAX_LEN); val.str[BTA_AG_AT_MAX_LEN] = 0; @@ -999,7 +1013,7 @@ void bta_ag_at_hsp_cback(tBTA_AG_SCB *p_scb, UINT16 cmd, UINT8 arg_type, ** *******************************************************************************/ 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_EVT event; @@ -1024,6 +1038,13 @@ void bta_ag_at_hfp_cback(tBTA_AG_SCB *p_scb, UINT16 cmd, UINT8 arg_type, val.hdr.app_id = p_scb->app_id; val.num = int_arg; bdcpy(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; + } BCM_STRNCPY_S(val.str, sizeof(val.str), p_arg, BTA_AG_AT_MAX_LEN); val.str[BTA_AG_AT_MAX_LEN] = 0; @@ -1178,7 +1199,7 @@ void bta_ag_at_hfp_cback(tBTA_AG_SCB *p_scb, UINT16 cmd, UINT8 arg_type, case BTA_AG_HF_CMD_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); @@ -1369,7 +1390,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 3e12dff4e..68def0307 100644 --- a/bta/ag/bta_ag_int.h +++ b/bta/ag/bta_ag_int.h @@ -389,9 +389,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); |