From 4633fb8d4b0376474f6d38c58ee5ee563a392ef5 Mon Sep 17 00:00:00 2001 From: Mallikarjuna GB Date: Fri, 7 Nov 2014 16:52:25 +0530 Subject: [PATCH] Fix issues in A2dp, Avrcp, HF and AG reported by static analysis tool - Fixes to validate Array Index Value and Null Pointer Dereference reported by static analysis tool. Change-Id: Id1492315f68378fdcfa517bd0a5cacefc8ebfddb --- bta/ag/bta_ag_at.c | 6 +++- bta/ag/bta_ag_cmd.c | 6 ++++ bta/av/bta_av_aact.c | 9 +++++- bta/av/bta_av_act.c | 35 ++++++++++++++++----- bta/av/bta_av_main.c | 7 +++-- btif/src/btif_hf.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++ stack/avct/avct_api.c | 2 +- stack/avdt/avdt_scb.c | 4 +-- stack/avrc/avrc_api.c | 2 +- 9 files changed, 140 insertions(+), 16 deletions(-) diff --git a/bta/ag/bta_ag_at.c b/bta/ag/bta_ag_at.c index 74d39482e..66480401a 100644 --- a/bta/ag/bta_ag_at.c +++ b/bta/ag/bta_ag_at.c @@ -192,7 +192,11 @@ void bta_ag_at_parse(tBTA_AG_AT_CB *p_cb, char *p_buf, UINT16 len) if (p_cb->p_cmd_buf == NULL) { - p_cb->p_cmd_buf = (char *) GKI_getbuf(p_cb->cmd_max_len); + if ((p_cb->p_cmd_buf = (char *) GKI_getbuf(p_cb->cmd_max_len)) == NULL) + { + APPL_TRACE_ERROR("%s: GKI_getbuf() failed allocation", __func__); + return; + } p_cb->cmd_pos = 0; } diff --git a/bta/ag/bta_ag_cmd.c b/bta/ag/bta_ag_cmd.c index 8546a9a34..cc5f3cf05 100644 --- a/bta/ag/bta_ag_cmd.c +++ b/bta/ag/bta_ag_cmd.c @@ -877,6 +877,12 @@ void bta_ag_at_hfp_cback(tBTA_AG_SCB *p_scb, UINT16 cmd, UINT8 arg_type, #if (BTM_WBS_INCLUDED == TRUE ) tBTA_AG_PEER_CODEC codec_type, codec_sent; #endif + if (p_arg == NULL) + { + APPL_TRACE_ERROR("%s: p_arg is null, send error and return", __func__); + bta_ag_send_error(p_scb, BTA_AG_ERR_INV_CHAR_IN_TSTR); + return; + } APPL_TRACE_DEBUG("HFP AT cmd:%d arg_type:%d arg:%d arg:%s", cmd, arg_type, int_arg, p_arg); diff --git a/bta/av/bta_av_aact.c b/bta/av/bta_av_aact.c index c394d66ea..ac44f0d30 100644 --- a/bta/av/bta_av_aact.c +++ b/bta/av/bta_av_aact.c @@ -556,7 +556,14 @@ static void bta_av_proc_stream_evt(UINT8 handle, BD_ADDR bd_addr, UINT8 event, t /* coverity[var_deref_model] */ /* false-positive: bta_av_conn_cback only processes AVDT_CONNECT_IND_EVT and AVDT_DISCONNECT_IND_EVT event * these 2 events always have associated p_data */ - bta_av_conn_cback(handle, bd_addr, event, p_data); + if (p_data) + { + bta_av_conn_cback(handle, bd_addr, event, p_data); + } + else + { + APPL_TRACE_ERROR("%s: p_data is null", __func__); + } } /******************************************************************************* diff --git a/bta/av/bta_av_act.c b/bta/av/bta_av_act.c index b41165f04..75e090097 100644 --- a/bta/av/bta_av_act.c +++ b/bta/av/bta_av_act.c @@ -96,11 +96,16 @@ void bta_av_del_rc(tBTA_AV_RCB *p_rcb) tBTA_AV_SCB *p_scb; UINT8 rc_handle; /* connected AVRCP handle */ + p_scb = NULL; if(p_rcb->handle != BTA_AV_RC_HANDLE_NONE) { if(p_rcb->shdl) { - p_scb = bta_av_cb.p_scb[p_rcb->shdl - 1]; + /* Validate array index*/ + if ((p_rcb->shdl - 1) < BTA_AV_NUM_STRS) + { + p_scb = bta_av_cb.p_scb[p_rcb->shdl - 1]; + } if(p_scb) { APPL_TRACE_DEBUG("bta_av_del_rc shdl:%d, srch:%d rc_handle:%d", p_rcb->shdl, @@ -1160,7 +1165,7 @@ void bta_av_stream_chg(tBTA_AV_SCB *p_scb, BOOLEAN started) void bta_av_conn_chg(tBTA_AV_DATA *p_data) { tBTA_AV_CB *p_cb = &bta_av_cb; - tBTA_AV_SCB *p_scb; + tBTA_AV_SCB *p_scb = NULL; tBTA_AV_SCB *p_scbi; UINT8 mask; UINT8 conn_msk; @@ -1172,8 +1177,11 @@ void bta_av_conn_chg(tBTA_AV_DATA *p_data) tBTA_AV_RCB *p_rcb, *p_rcb2; BOOLEAN chk_restore = FALSE; - p_scb = p_cb->p_scb[index]; - + /* Validate array index*/ + if (index < BTA_AV_NUM_STRS) + { + p_scb = p_cb->p_scb[index]; + } mask = BTA_AV_HNDL_TO_MSK(index); p_lcb = bta_av_find_lcb(p_data->conn_chg.peer_addr, BTA_AV_LCB_FIND); conn_msk = 1 << (index + 1); @@ -1558,9 +1566,12 @@ static void bta_av_acp_sig_timer_cback (TIMER_LIST_ENT *p_tle) { UINT8 inx = (UINT8)p_tle->param; tBTA_AV_CB *p_cb = &bta_av_cb; - tBTA_AV_SCB *p_scb = p_cb->p_scb[inx]; + tBTA_AV_SCB *p_scb = NULL; tBTA_AV_API_OPEN *p_buf; - + if (inx < BTA_AV_NUM_STRS) + { + p_scb = p_cb->p_scb[inx]; + } if (p_scb) { APPL_TRACE_DEBUG("bta_av_acp_sig_timer_cback, coll_mask = 0x%02X", p_scb->coll_mask); @@ -1710,7 +1721,11 @@ void bta_av_rc_disc_done(tBTA_AV_DATA *p_data) } else { - p_scb = p_cb->p_scb[(p_cb->disc & BTA_AV_HNDL_MSK) - 1]; + /* Validate array index*/ + if (((p_cb->disc & BTA_AV_HNDL_MSK) - 1) < BTA_AV_NUM_STRS) + { + p_scb = p_cb->p_scb[(p_cb->disc & BTA_AV_HNDL_MSK) - 1]; + } if (p_scb) rc_handle = p_scb->rc_handle; else @@ -1798,6 +1813,7 @@ void bta_av_rc_closed(tBTA_AV_DATA *p_data) tBTA_AV_LCB *p_lcb; rc_close.rc_handle = BTA_AV_RC_HANDLE_NONE; + p_scb = NULL; APPL_TRACE_DEBUG("bta_av_rc_closed rc_handle:%d", p_msg->handle); for(i=0; ishdl, p_rcb->lidx); if(p_rcb->shdl) { - p_scb = bta_av_cb.p_scb[p_rcb->shdl - 1]; + if ((p_rcb->shdl - 1) < BTA_AV_NUM_STRS) + { + p_scb = bta_av_cb.p_scb[p_rcb->shdl - 1]; + } if(p_scb) { bdcpy(rc_close.peer_addr, p_scb->peer_addr); diff --git a/bta/av/bta_av_main.c b/bta/av/bta_av_main.c index 849dd1ee9..9a409292e 100644 --- a/bta/av/bta_av_main.c +++ b/bta/av/bta_av_main.c @@ -935,7 +935,7 @@ void bta_av_restore_switch (void) static void bta_av_sys_rs_cback (tBTA_SYS_CONN_STATUS status,UINT8 id, UINT8 app_id, BD_ADDR peer_addr) { int i; - tBTA_AV_SCB *p_scb; + tBTA_AV_SCB *p_scb = NULL; tBTA_AV_ROLE_RES *p_buf; UINT8 cur_role; UINT8 peer_idx = 0; @@ -979,7 +979,10 @@ static void bta_av_sys_rs_cback (tBTA_SYS_CONN_STATUS status,UINT8 id, UINT8 app /* we need to continue opening process for the BTA_AvOpen(). */ if ((bta_av_cb.rs_idx != 0) && (bta_av_cb.rs_idx != peer_idx)) { - p_scb = bta_av_cb.p_scb[bta_av_cb.rs_idx - 1]; + if ((bta_av_cb.rs_idx -1) < BTA_AV_NUM_STRS) + { + p_scb = bta_av_cb.p_scb[bta_av_cb.rs_idx - 1]; + } if (p_scb && p_scb->q_tag == BTA_AV_Q_TAG_OPEN) { APPL_TRACE_DEBUG ("bta_av_sys_rs_cback: rs_idx(%d), hndl:x%x q_tag: %d", diff --git a/btif/src/btif_hf.c b/btif/src/btif_hf.c index f3fd9833b..69b76a5d4 100644 --- a/btif/src/btif_hf.c +++ b/btif/src/btif_hf.c @@ -394,6 +394,12 @@ static void btif_hf_upstreams_evt(UINT16 event, char* p_param) BTIF_TRACE_DEBUG("%s: event=%s", __FUNCTION__, dump_hf_event(event)); + if ((idx < 0) || (idx >= BTIF_HF_NUM_CB)) + { + BTIF_TRACE_ERROR("%s: Invalid index %d", __FUNCTION__, idx); + return; + } + switch (event) { case BTA_AG_ENABLE_EVT: @@ -672,6 +678,13 @@ static void btif_in_hf_generic_evt(UINT16 event, char *p_param) int idx = btif_hf_idx_by_bdaddr((bt_bdaddr_t *)p_param); BTIF_TRACE_EVENT("%s: event=%d", __FUNCTION__, event); + + if ((idx < 0) || (idx >= BTIF_HF_NUM_CB)) + { + BTIF_TRACE_ERROR("%s: Invalid index %d", __FUNCTION__, idx); + return; + } + switch (event) { case BTIF_HFP_CB_AUDIO_CONNECTING: { @@ -783,6 +796,12 @@ static bt_status_t disconnect( bt_bdaddr_t *bd_addr ) int idx = btif_hf_idx_by_bdaddr(bd_addr); + if ((idx < 0) || (idx >= BTIF_HF_NUM_CB)) + { + BTIF_TRACE_ERROR("%s: Invalid index %d", __FUNCTION__, idx); + return BT_STATUS_FAIL; + } + if (is_connected(bd_addr) && (idx != BTIF_HF_INVALID_IDX)) { BTA_AgClose(btif_hf_cb[idx].handle); @@ -807,6 +826,12 @@ static bt_status_t connect_audio( bt_bdaddr_t *bd_addr ) int idx = btif_hf_idx_by_bdaddr(bd_addr); + if ((idx < 0) || (idx >= BTIF_HF_NUM_CB)) + { + BTIF_TRACE_ERROR("%s: Invalid index %d", __FUNCTION__, idx); + return BT_STATUS_FAIL; + } + /* Check if SLC is connected */ if (btif_hf_check_if_slc_connected() != BT_STATUS_SUCCESS) return BT_STATUS_NOT_READY; @@ -839,6 +864,12 @@ static bt_status_t disconnect_audio( bt_bdaddr_t *bd_addr ) int idx = btif_hf_idx_by_bdaddr(bd_addr); + if ((idx < 0) || (idx >= BTIF_HF_NUM_CB)) + { + BTIF_TRACE_ERROR("%s: Invalid index %d", __FUNCTION__, idx); + return BT_STATUS_FAIL; + } + if (is_connected(bd_addr) && (idx != BTIF_HF_INVALID_IDX)) { BTA_AgAudioClose(btif_hf_cb[idx].handle); @@ -863,6 +894,12 @@ static bt_status_t start_voice_recognition(bt_bdaddr_t *bd_addr) int idx = btif_hf_idx_by_bdaddr(bd_addr); + if ((idx < 0) || (idx >= BTIF_HF_NUM_CB)) + { + BTIF_TRACE_ERROR("%s: Invalid index %d", __FUNCTION__, idx); + return BT_STATUS_FAIL; + } + if (is_connected(bd_addr) && (idx != BTIF_HF_INVALID_IDX)) { if (btif_hf_cb[idx].peer_feat & BTA_AG_PEER_FEAT_VREC) @@ -898,6 +935,12 @@ static bt_status_t stop_voice_recognition(bt_bdaddr_t *bd_addr) int idx = btif_hf_idx_by_bdaddr(bd_addr); + if ((idx < 0) || (idx >= BTIF_HF_NUM_CB)) + { + BTIF_TRACE_ERROR("%s: Invalid index %d", __FUNCTION__, idx); + return BT_STATUS_FAIL; + } + if (is_connected(bd_addr) && (idx != BTIF_HF_INVALID_IDX)) { if (btif_hf_cb[idx].peer_feat & BTA_AG_PEER_FEAT_VREC) @@ -934,6 +977,12 @@ static bt_status_t volume_control(bthf_volume_type_t type, int volume, int idx = btif_hf_idx_by_bdaddr(bd_addr); + if ((idx < 0) || (idx >= BTIF_HF_NUM_CB)) + { + BTIF_TRACE_ERROR("%s: Invalid index %d", __FUNCTION__, idx); + return BT_STATUS_FAIL; + } + tBTA_AG_RES_DATA ag_res; memset(&ag_res, 0, sizeof(tBTA_AG_RES_DATA)); if (is_connected(bd_addr) && (idx != BTIF_HF_INVALID_IDX)) @@ -994,6 +1043,12 @@ static bt_status_t cops_response(const char *cops, bt_bdaddr_t *bd_addr) int idx = btif_hf_idx_by_bdaddr(bd_addr); + if ((idx < 0) || (idx >= BTIF_HF_NUM_CB)) + { + BTIF_TRACE_ERROR("%s: Invalid index %d", __FUNCTION__, idx); + return BT_STATUS_FAIL; + } + if (is_connected(bd_addr) && (idx != BTIF_HF_INVALID_IDX)) { tBTA_AG_RES_DATA ag_res; @@ -1026,6 +1081,12 @@ static bt_status_t cind_response(int svc, int num_active, int num_held, int idx = btif_hf_idx_by_bdaddr(bd_addr); + if ((idx < 0) || (idx >= BTIF_HF_NUM_CB)) + { + BTIF_TRACE_ERROR("%s: Invalid index %d", __FUNCTION__, idx); + return BT_STATUS_FAIL; + } + if (is_connected(bd_addr) && (idx != BTIF_HF_INVALID_IDX)) { tBTA_AG_RES_DATA ag_res; @@ -1066,6 +1127,12 @@ static bt_status_t formatted_at_response(const char *rsp, bt_bdaddr_t *bd_addr) tBTA_AG_RES_DATA ag_res; int idx = btif_hf_idx_by_bdaddr(bd_addr); + if ((idx < 0) || (idx >= BTIF_HF_NUM_CB)) + { + BTIF_TRACE_ERROR("%s: Invalid index %d", __FUNCTION__, idx); + return BT_STATUS_FAIL; + } + if (is_connected(bd_addr) && (idx != BTIF_HF_INVALID_IDX)) { /* Format the response and send */ @@ -1095,6 +1162,12 @@ static bt_status_t at_response(bthf_at_response_t response_code, int idx = btif_hf_idx_by_bdaddr(bd_addr); + if ((idx < 0) || (idx >= BTIF_HF_NUM_CB)) + { + BTIF_TRACE_ERROR("%s: Invalid index %d", __FUNCTION__, idx); + return BT_STATUS_FAIL; + } + if (is_connected(bd_addr) && (idx != BTIF_HF_INVALID_IDX)) { send_at_result((response_code == BTHF_AT_RESPONSE_OK) ? BTA_AG_OK_DONE @@ -1126,6 +1199,12 @@ static bt_status_t clcc_response(int index, bthf_call_direction_t dir, int idx = btif_hf_idx_by_bdaddr(bd_addr); + if ((idx < 0) || (idx >= BTIF_HF_NUM_CB)) + { + BTIF_TRACE_ERROR("%s: Invalid index %d", __FUNCTION__, idx); + return BT_STATUS_FAIL; + } + if (is_connected(bd_addr) && (idx != BTIF_HF_INVALID_IDX)) { tBTA_AG_RES_DATA ag_res; @@ -1451,6 +1530,12 @@ static bt_status_t configure_wbs( bt_bdaddr_t *bd_addr , bthf_wbs_config_t conf int idx = btif_hf_idx_by_bdaddr(bd_addr); + if ((idx < 0) || (idx >= BTIF_HF_NUM_CB)) + { + BTIF_TRACE_ERROR("%s: Invalid index %d", __FUNCTION__, idx); + return BT_STATUS_FAIL; + } + BTIF_TRACE_EVENT("%s config is %d", __FUNCTION__,config); if (config == BTHF_WBS_YES) BTA_AgSetCodec(btif_hf_cb[idx].handle,BTA_AG_CODEC_MSBC); diff --git a/stack/avct/avct_api.c b/stack/avct/avct_api.c index 5ee928228..bf3dd1854 100644 --- a/stack/avct/avct_api.c +++ b/stack/avct/avct_api.c @@ -262,7 +262,7 @@ UINT16 AVCT_CreateBrowse (UINT8 handle, UINT8 role) if (role == AVCT_INT) { /* the link control block must exist before this function is called as INT. */ - if (p_ccb->p_lcb == NULL) + if ((p_ccb->p_lcb == NULL) || (p_ccb->p_lcb->allocated == 0)) { result = AVCT_NOT_OPEN; } diff --git a/stack/avdt/avdt_scb.c b/stack/avdt/avdt_scb.c index f5f5b5a79..696fccabf 100644 --- a/stack/avdt/avdt_scb.c +++ b/stack/avdt/avdt_scb.c @@ -740,7 +740,7 @@ UINT8 avdt_scb_verify(tAVDT_CCB *p_ccb, UINT8 state, UINT8 *p_seid, UINT16 num_s nsc_mask = AVDT_NSC_SUSPEND; /* verify every scb */ - for (i = 0, *p_err_code = 0; i < num_seid && *p_err_code == 0; i++) + for (i = 0, *p_err_code = 0; (i < num_seid) && (*p_err_code == 0) && (i < AVDT_NUM_SEPS); i++) { if ((p_scb = avdt_scb_by_hdl(p_seid[i])) == NULL) *p_err_code = AVDT_ERR_BAD_STATE; @@ -764,7 +764,7 @@ UINT8 avdt_scb_verify(tAVDT_CCB *p_ccb, UINT8 state, UINT8 *p_seid, UINT16 num_s } } - if (i != num_seid) + if ((i != num_seid) && (i < AVDT_NUM_SEPS)) { ret = p_seid[i]; } diff --git a/stack/avrc/avrc_api.c b/stack/avrc/avrc_api.c index bee9aa8da..5afa80fe6 100644 --- a/stack/avrc/avrc_api.c +++ b/stack/avrc/avrc_api.c @@ -1016,7 +1016,7 @@ UINT16 AVRC_MsgReq (UINT8 handle, UINT8 label, UINT8 ctype, BT_HDR *p_pkt) AVRC_TRACE_DEBUG ("p_pkt->len(%d) > AVRC_MAX_CTRL_DATA_LEN", p_pkt->len ); p_pkt_new = (BT_HDR *)GKI_getbuf((UINT16)(AVRC_PACKET_LEN + AVCT_MSG_OFFSET + BT_HDR_SIZE)); - if (p_pkt_new) + if (p_pkt_new && (p_start != NULL)) { p_fcb->frag_enabled = TRUE; p_fcb->p_fmsg = p_pkt; -- 2.11.0