From eb1c77472adf30dbd7dc3df7c60c941cafff84d2 Mon Sep 17 00:00:00 2001 From: Jin Cheng Date: Mon, 23 Mar 2026 11:10:00 +0800 Subject: [PATCH] fix(bt/bluedroid): fixed multiple high-severity issues from AI code review in SPP --- .../bt/host/bluedroid/api/esp_spp_api.c | 11 +++---- .../bt/host/bluedroid/bta/jv/bta_jv_act.c | 11 ++++--- .../bt/host/bluedroid/bta/jv/bta_jv_api.c | 19 ++++++++++-- .../bt/host/bluedroid/stack/goep/goepc_main.c | 1 + .../bt/host/bluedroid/stack/obex/obex_api.c | 23 ++++++++++++-- .../bluedroid/stack/rfcomm/rfc_port_fsm.c | 6 ++-- .../bluedroid/stack/rfcomm/rfc_ts_frames.c | 31 +++++++++++++++++++ .../host/bluedroid/stack/rfcomm/rfc_utils.c | 17 +++++----- 8 files changed, 93 insertions(+), 26 deletions(-) diff --git a/components/bt/host/bluedroid/api/esp_spp_api.c b/components/bt/host/bluedroid/api/esp_spp_api.c index 18ef661c165..c739bb0cd2b 100644 --- a/components/bt/host/bluedroid/api/esp_spp_api.c +++ b/components/bt/host/bluedroid/api/esp_spp_api.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -70,14 +70,13 @@ esp_err_t esp_spp_enhanced_init(const esp_spp_cfg_t *cfg) esp_err_t esp_spp_deinit(void) { btc_msg_t msg; - btc_spp_args_t arg; ESP_BLUEDROID_STATUS_CHECK(ESP_BLUEDROID_STATUS_ENABLED); msg.sig = BTC_SIG_API_CALL; msg.pid = BTC_PID_SPP; msg.act = BTC_SPP_ACT_UNINIT; - return (btc_transfer_context(&msg, &arg, sizeof(btc_spp_args_t), NULL, NULL) == BT_STATUS_SUCCESS ? ESP_OK : ESP_FAIL); + return (btc_transfer_context(&msg, NULL, 0, NULL, NULL) == BT_STATUS_SUCCESS ? ESP_OK : ESP_FAIL); } @@ -242,27 +241,25 @@ esp_err_t esp_spp_write(uint32_t handle, int len, uint8_t *p_data) esp_err_t esp_spp_vfs_register(void) { btc_msg_t msg; - btc_spp_args_t arg; ESP_BLUEDROID_STATUS_CHECK(ESP_BLUEDROID_STATUS_ENABLED); msg.sig = BTC_SIG_API_CALL; msg.pid = BTC_PID_SPP; msg.act = BTC_SPP_ACT_VFS_REGISTER; - return (btc_transfer_context(&msg, &arg, sizeof(btc_spp_args_t), NULL, NULL) == BT_STATUS_SUCCESS ? ESP_OK : ESP_FAIL); + return (btc_transfer_context(&msg, NULL, 0, NULL, NULL) == BT_STATUS_SUCCESS ? ESP_OK : ESP_FAIL); } esp_err_t esp_spp_vfs_unregister(void) { btc_msg_t msg; - btc_spp_args_t arg; ESP_BLUEDROID_STATUS_CHECK(ESP_BLUEDROID_STATUS_ENABLED); msg.sig = BTC_SIG_API_CALL; msg.pid = BTC_PID_SPP; msg.act = BTC_SPP_ACT_VFS_UNREGISTER; - return (btc_transfer_context(&msg, &arg, sizeof(btc_spp_args_t), NULL, NULL) == BT_STATUS_SUCCESS ? ESP_OK : ESP_FAIL); + return (btc_transfer_context(&msg, NULL, 0, NULL, NULL) == BT_STATUS_SUCCESS ? ESP_OK : ESP_FAIL); } esp_err_t esp_spp_get_profile_status(esp_spp_profile_status_t *profile_status) diff --git a/components/bt/host/bluedroid/bta/jv/bta_jv_act.c b/components/bt/host/bluedroid/bta/jv/bta_jv_act.c index bf0d91545b4..991803df0ac 100644 --- a/components/bt/host/bluedroid/bta/jv/bta_jv_act.c +++ b/components/bt/host/bluedroid/bta/jv/bta_jv_act.c @@ -288,8 +288,7 @@ tBTA_JV_RFC_CB *bta_jv_rfc_port_to_cb(UINT16 port_handle) p_cb = &bta_jv_cb.rfc_cb[handle - 1]; } } else { - APPL_TRACE_WARNING("bta_jv_rfc_port_to_cb(port_handle:0x%x):jv handle:0x%x not" - " FOUND", port_handle, bta_jv_cb.port_cb[port_handle - 1].handle); + APPL_TRACE_WARNING("bta_jv_rfc_port_to_cb(port_handle:0x%x)", port_handle); } return p_cb; } @@ -993,8 +992,11 @@ static void bta_jv_start_discovery_cback(UINT16 result, void *user_data) } else { dcomp.service_name[dcomp.scn_num] = NULL; } - dcomp.scn_num++; status = BTA_JV_SUCCESS; + dcomp.scn_num++; + if (dcomp.scn_num == BTA_JV_MAX_SCN) { + break; + } } } while (p_sdp_rec); } @@ -2860,6 +2862,7 @@ static void fcchan_conn_chng_cbk(UINT16 chan, BD_ADDR bd_addr, BOOLEAN connected open_evt.l2c_open.status = BTA_JV_SUCCESS; } else { fcclient_free(t); + t = NULL; open_evt.l2c_open.status = BTA_JV_FAILURE; } } @@ -2871,7 +2874,7 @@ static void fcchan_conn_chng_cbk(UINT16 chan, BD_ADDR bd_addr, BOOLEAN connected //call this with lock taken so socket does not disappear from under us */ if (p_cback) { p_cback(BTA_JV_L2CAP_OPEN_EVT, &open_evt, user_data); - if (!t->p_cback) { /* no callback set, means they do not want this one... */ + if (t && !t->p_cback) { /* no callback set, means they do not want this one... */ fcclient_free(t); } } diff --git a/components/bt/host/bluedroid/bta/jv/bta_jv_api.c b/components/bt/host/bluedroid/bta/jv/bta_jv_api.c index 79518e6917c..c83f9b4e8f6 100644 --- a/components/bt/host/bluedroid/bta/jv/bta_jv_api.c +++ b/components/bt/host/bluedroid/bta/jv/bta_jv_api.c @@ -74,6 +74,14 @@ tBTA_JV_STATUS BTA_JvEnable(tBTA_JV_DM_CBACK *p_cback) p_bta_jv_cfg->p_sdp_raw_data = (UINT8 *)osi_malloc(p_bta_jv_cfg->sdp_raw_size); p_bta_jv_cfg->p_sdp_db = (tSDP_DISCOVERY_DB *)osi_malloc(p_bta_jv_cfg->sdp_db_size); if (p_bta_jv_cfg->p_sdp_raw_data == NULL || p_bta_jv_cfg->p_sdp_db == NULL) { + if (p_bta_jv_cfg->p_sdp_raw_data) { + osi_free(p_bta_jv_cfg->p_sdp_raw_data); + p_bta_jv_cfg->p_sdp_raw_data = NULL; + } + if (p_bta_jv_cfg->p_sdp_db) { + osi_free( p_bta_jv_cfg->p_sdp_db); + p_bta_jv_cfg->p_sdp_db = NULL; + } return BTA_JV_NO_DATA; } #endif @@ -287,7 +295,9 @@ tBTA_JV_STATUS BTA_JvStartDiscovery(BD_ADDR bd_addr, UINT16 num_uuid, p_msg->hdr.event = BTA_JV_API_START_DISCOVERY_EVT; bdcpy(p_msg->bd_addr, bd_addr); p_msg->num_uuid = num_uuid; - memcpy(p_msg->uuid_list, p_uuid_list, num_uuid * sizeof(tSDP_UUID)); + if (p_uuid_list && (num_uuid > 0)) { + memcpy(p_msg->uuid_list, p_uuid_list, num_uuid * sizeof(tSDP_UUID)); + } p_msg->num_attr = 0; p_msg->user_data = user_data; bta_sys_sendmsg(p_msg); @@ -318,7 +328,12 @@ tBTA_JV_STATUS BTA_JvCreateRecordByUser(const char *name, UINT32 channel, void * if ((p_msg = (tBTA_JV_API_CREATE_RECORD *)osi_malloc(sizeof(tBTA_JV_API_CREATE_RECORD))) != NULL) { p_msg->hdr.event = BTA_JV_API_CREATE_RECORD_EVT; p_msg->user_data = user_data; - strcpy(p_msg->name, name); + if (name) { + strncpy(p_msg->name, name, ESP_SDP_SERVER_NAME_MAX); + p_msg->name[ESP_SDP_SERVER_NAME_MAX] = '\0'; + } else { + p_msg->name[0] = '\0'; + } p_msg->channel = channel; bta_sys_sendmsg(p_msg); status = BTA_JV_SUCCESS; diff --git a/components/bt/host/bluedroid/stack/goep/goepc_main.c b/components/bt/host/bluedroid/stack/goep/goepc_main.c index e0859580ec6..75b83919f37 100644 --- a/components/bt/host/bluedroid/stack/goep/goepc_main.c +++ b/components/bt/host/bluedroid/stack/goep/goepc_main.c @@ -80,6 +80,7 @@ static tGOEPC_CCB *find_ccb_by_obex_handle(UINT16 obex_handle) for (int i = 0; i < GOEPC_MAX_CONNECTION; ++i) { if (goepc_cb.ccb[i].allocated && goepc_cb.ccb[i].obex_handle == obex_handle) { p_ccb = &goepc_cb.ccb[i]; + break; } } return p_ccb; diff --git a/components/bt/host/bluedroid/stack/obex/obex_api.c b/components/bt/host/bluedroid/stack/obex/obex_api.c index 99276a93cf1..dedaa8c8b6f 100644 --- a/components/bt/host/bluedroid/stack/obex/obex_api.c +++ b/components/bt/host/bluedroid/stack/obex/obex_api.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -121,7 +121,7 @@ UINT16 OBEX_CreateConn(tOBEX_SVR_INFO *server, tOBEX_MSG_CBACK callback, UINT16 tOBEX_CCB *p_ccb = NULL; do { - if (server->tl >= OBEX_NUM_TL) { + if (!server || (server->tl >= OBEX_NUM_TL)) { ret = OBEX_INVALID_PARAM; break; } @@ -144,7 +144,9 @@ UINT16 OBEX_CreateConn(tOBEX_SVR_INFO *server, tOBEX_MSG_CBACK callback, UINT16 p_ccb->callback = callback; p_ccb->role = OBEX_ROLE_CLIENT; p_ccb->state = OBEX_STATE_OPENING; - *out_handle = p_ccb->allocated; + if (out_handle) { + *out_handle = p_ccb->allocated; + } } while (0); if (ret != OBEX_SUCCESS && p_ccb != NULL) { @@ -627,20 +629,35 @@ UINT16 OBEX_ParseRequest(BT_HDR *pkt, tOBEX_PARSE_INFO *info) } UINT8 *p_data = (UINT8 *)(pkt + 1) + pkt->offset; + UINT16 len = pkt->len; + + if (len < 1) { + return OBEX_FAILURE; + } + info->opcode = *p_data; switch (info->opcode) { case OBEX_OPCODE_CONNECT: + if (len < 7) { + return OBEX_FAILURE; + } info->obex_version_number = p_data[3]; info->flags = p_data[4]; info->max_packet_length = (p_data[5] << 8) + p_data[6]; info->next_header_pos = 7; break; case OBEX_OPCODE_SETPATH: + if (len < 5) { + return OBEX_FAILURE; + } info->flags = p_data[3]; info->next_header_pos = 5; break; default: + if (len < 3) { + return OBEX_FAILURE; + } info->next_header_pos = 3; break; } diff --git a/components/bt/host/bluedroid/stack/rfcomm/rfc_port_fsm.c b/components/bt/host/bluedroid/stack/rfcomm/rfc_port_fsm.c index 8d8fe3cac66..a47ade5b4ab 100644 --- a/components/bt/host/bluedroid/stack/rfcomm/rfc_port_fsm.c +++ b/components/bt/host/bluedroid/stack/rfcomm/rfc_port_fsm.c @@ -62,13 +62,13 @@ static void rfc_set_port_state(tPORT_STATE *port_pars, MX_FRAME *p_frame); *******************************************************************************/ void rfc_port_sm_execute (tPORT *p_port, UINT16 event, void *p_data) { - RFCOMM_TRACE_DEBUG("%s st:%d, evt:%d\n", __func__, p_port->rfc.state, event); - if (!p_port) { RFCOMM_TRACE_WARNING ("NULL port event %d", event); return; } + RFCOMM_TRACE_DEBUG("%s st:%d, evt:%d\n", __func__, p_port->rfc.state, event); + switch (p_port->rfc.state) { case RFC_STATE_CLOSED: rfc_port_sm_state_closed (p_port, event, p_data); @@ -240,7 +240,7 @@ void rfc_port_sm_sabme_wait_ua (tPORT *p_port, UINT16 event, void *p_data) ** ** Description This function handles events for the port in the ** WAIT_SEC_CHECK state. SABME has been received from the -** peer and Security Manager verifes BD_ADDR, before we can +** peer and Security Manager verifies BD_ADDR, before we can ** send ESTABLISH_IND to the Port entity ** ** Returns void diff --git a/components/bt/host/bluedroid/stack/rfcomm/rfc_ts_frames.c b/components/bt/host/bluedroid/stack/rfcomm/rfc_ts_frames.c index 2d06823173f..cbd6e7de8c0 100644 --- a/components/bt/host/bluedroid/stack/rfcomm/rfc_ts_frames.c +++ b/components/bt/host/bluedroid/stack/rfcomm/rfc_ts_frames.c @@ -179,8 +179,17 @@ void rfc_send_buf_uih (tRFC_MCB *p_mcb, UINT8 dlci, BT_HDR *p_buf) UINT8 cr = RFCOMM_CR(p_mcb->is_initiator, TRUE); UINT8 credits; + if (p_buf->offset < RFCOMM_CTRL_FRAME_LEN) { + osi_free(p_buf); + return; + } + p_buf->offset -= RFCOMM_CTRL_FRAME_LEN; if (p_buf->len > 127) { + if (p_buf->offset < 1) { + osi_free(p_buf); + return; + } p_buf->offset--; } @@ -191,6 +200,10 @@ void rfc_send_buf_uih (tRFC_MCB *p_mcb, UINT8 dlci, BT_HDR *p_buf) } if (credits) { + if (p_buf->offset < 1) { + osi_free(p_buf); + return; + } p_buf->offset--; } @@ -558,8 +571,26 @@ void rfc_send_test (tRFC_MCB *p_mcb, BOOLEAN is_command, BT_HDR *p_buf) UINT16 xx; UINT8 *p_src, *p_dest; + if (p_buf->offset + sizeof(BT_HDR) >= RFCOMM_CMD_BUF_SIZE) { + osi_free(p_buf); + return; + } + + UINT16 max_len = RFCOMM_CMD_BUF_SIZE - sizeof(BT_HDR) - p_buf->offset; + if (p_buf->offset < (L2CAP_MIN_OFFSET + RFCOMM_MIN_OFFSET + 2)) { + if (max_len < (L2CAP_MIN_OFFSET + RFCOMM_MIN_OFFSET + 2 - p_buf->offset)) { + osi_free(p_buf); + return; + } + max_len -= (L2CAP_MIN_OFFSET + RFCOMM_MIN_OFFSET + 2 - p_buf->offset); + } + if (p_buf->len > max_len) { + p_buf->len = max_len; + } + BT_HDR *p_buf_new; if ((p_buf_new = (BT_HDR *)osi_malloc(RFCOMM_CMD_BUF_SIZE)) == NULL) { + osi_free(p_buf); return; } memcpy(p_buf_new, p_buf, sizeof(BT_HDR) + p_buf->offset + p_buf->len); diff --git a/components/bt/host/bluedroid/stack/rfcomm/rfc_utils.c b/components/bt/host/bluedroid/stack/rfcomm/rfc_utils.c index b766918d0e8..ee5e1feb568 100644 --- a/components/bt/host/bluedroid/stack/rfcomm/rfc_utils.c +++ b/components/bt/host/bluedroid/stack/rfcomm/rfc_utils.c @@ -491,18 +491,21 @@ void rfc_check_send_cmd(tRFC_MCB *p_mcb, BT_HDR *p_buf) RFCOMM_TRACE_ERROR("%s: empty queue: p_mcb = %p p_mcb->lcid = %u cached p_mcb = %p", __func__, p_mcb, p_mcb->lcid, rfc_find_lcid_mcb(p_mcb->lcid)); + osi_free(p_buf); + } else { + fixed_queue_enqueue(p_mcb->cmd_q, p_buf, FIXED_QUEUE_MAX_TIMEOUT); } - fixed_queue_enqueue(p_mcb->cmd_q, p_buf, FIXED_QUEUE_MAX_TIMEOUT); } /* handle queue if L2CAP not congested */ - while (p_mcb->l2cap_congested == FALSE) { - if ((p = (BT_HDR *)fixed_queue_dequeue(p_mcb->cmd_q, 0)) == NULL) { - break; + if (p_mcb->cmd_q) { + while (p_mcb->l2cap_congested == FALSE) { + if ((p = (BT_HDR *)fixed_queue_dequeue(p_mcb->cmd_q, 0)) == NULL) { + break; + } + + L2CA_DataWrite (p_mcb->lcid, p); } - - - L2CA_DataWrite (p_mcb->lcid, p); } }