From 843673ada2e33301376edc373f96b17460244fc1 Mon Sep 17 00:00:00 2001 From: Shreeyash Bhakare Date: Thu, 11 Jun 2026 16:59:52 +0530 Subject: [PATCH] fix(nimble): Ai reported security fixes --- nimble/host/src/ble_att_svr.c | 26 +++++++++++-------- nimble/host/src/ble_l2cap_sig.c | 38 +++++++++++----------------- nimble/host/src/ble_sm.c | 7 ++++-- nimble/host/src/ble_sm_sc.c | 44 +++++++++++++++++++++++---------- 4 files changed, 67 insertions(+), 48 deletions(-) diff --git a/nimble/host/src/ble_att_svr.c b/nimble/host/src/ble_att_svr.c index f12dc0558..6f0c76e12 100644 --- a/nimble/host/src/ble_att_svr.c +++ b/nimble/host/src/ble_att_svr.c @@ -2658,16 +2658,21 @@ ble_att_svr_rx_signed_write(uint16_t conn_handle, uint16_t cid, struct os_mbuf * /* Converting cmac to little endian */ swap_in_place(cmac, sizeof cmac); - /* Comparing sign counter */ - if(memcmp(sign, &value_sec.sign_counter, sizeof(value_sec.sign_counter)) != 0) { - rc = BLE_HS_EAUTHEN; - goto err; - } - - /* Comparing signature */ - if(memcmp(&sign[sizeof(value_sec.sign_counter)], &cmac[sizeof(cmac) / 2], sizeof(cmac) / 2) != 0) { - rc = BLE_HS_EAUTHEN; - goto err; + /* Comparing sign counter and signature using constant-time comparison */ + { + uint8_t diff = 0; + const uint8_t *counter_bytes = (const uint8_t *)&value_sec.sign_counter; + for (size_t j = 0; j < sizeof(value_sec.sign_counter); j++) { + diff |= sign[j] ^ counter_bytes[j]; + } + for (size_t j = 0; j < sizeof(cmac) / 2; j++) { + diff |= sign[sizeof(value_sec.sign_counter) + j] ^ + cmac[sizeof(cmac) / 2 + j]; + } + if (diff != 0) { + rc = BLE_HS_EAUTHEN; + goto err; + } } #if MYNEWT_VAL(BLE_SM_SIGN_CNT) @@ -3238,6 +3243,7 @@ ble_att_svr_rx_notify_multi(uint16_t conn_handle, uint16_t cid, struct os_mbuf * attr_len = le16toh(req->value_len); os_mbuf_adj(*rxom, 4); + pkt_len = OS_MBUF_PKTLEN(*rxom); if (attr_len > BLE_ATT_ATTR_MAX_LEN) { BLE_HS_LOG_ERROR("attr length (%d) > max (%d)", diff --git a/nimble/host/src/ble_l2cap_sig.c b/nimble/host/src/ble_l2cap_sig.c index 7fdf6c853..0a52fd9c1 100644 --- a/nimble/host/src/ble_l2cap_sig.c +++ b/nimble/host/src/ble_l2cap_sig.c @@ -931,11 +931,13 @@ ble_l2cap_sig_credit_base_reconfig_req_rx(uint16_t conn_handle, goto failed; } - ble_hs_unlock(); - for (i = 0; i < cid_cnt; i++) { chan[i]->coc_tx.mtu = host_mtu; chan[i]->peer_coc_mps = host_mps; + } + ble_hs_unlock(); + + for (i = 0; i < cid_cnt; i++) { ble_l2cap_event_coc_reconfigured(conn_handle, 0, chan[i], true); } @@ -1151,8 +1153,10 @@ ble_l2cap_sig_credit_base_con_req_rx(uint16_t conn_handle, chans[i]->cb = NULL; ble_hs_lock(); - conn = ble_hs_conn_find_assert(conn_handle); - ble_hs_conn_delete_chan(conn, chans[i]); + conn = ble_hs_conn_find(conn_handle); + if (conn) { + ble_hs_conn_delete_chan(conn, chans[i]); + } chans[i] = NULL; rsp->result = htole16(ble_l2cap_sig_ble_hs_err2coc_err(rc)); rc = 0; @@ -1160,7 +1164,13 @@ ble_l2cap_sig_credit_base_con_req_rx(uint16_t conn_handle, } ble_hs_lock(); - conn = ble_hs_conn_find_assert(conn_handle); + conn = ble_hs_conn_find(conn_handle); + if (!conn) { + for (int j = 0; j < num_of_scids; j++) { + chans[j] = NULL; + } + goto done; + } } done: @@ -1221,7 +1231,6 @@ ble_l2cap_sig_credit_base_con_rsp_rx(uint16_t conn_handle, struct ble_hs_conn *conn; int rc = 0; int i; - uint16_t duplicated_cids[BLE_L2CAP_MAX_COC_CONN_REQ] = {0}; #if !BLE_MONITOR BLE_HS_LOG(DEBUG, "L2CAP LE COC connection response received\n"); @@ -1281,7 +1290,6 @@ ble_l2cap_sig_credit_base_con_rsp_rx(uint16_t conn_handle, uint16_t peer_dcid = le16toh(rsp->dcids[i]); if (ble_hs_conn_chan_find_by_dcid(conn, peer_dcid)) { - duplicated_cids[i] = peer_dcid; chan->dcid = 0; continue; } @@ -1297,22 +1305,6 @@ ble_l2cap_sig_credit_base_con_rsp_rx(uint16_t conn_handle, ble_hs_unlock(); done: - for (i = 0; i < BLE_L2CAP_MAX_COC_CONN_REQ; i++) { - if (duplicated_cids[i] != 0){ - ble_hs_lock(); - conn = ble_hs_conn_find(conn_handle); - if (conn != NULL) { - chan = ble_hs_conn_chan_find_by_dcid(conn, duplicated_cids[i]); - } else { - chan = NULL; - } - ble_hs_unlock(); - if (chan != NULL) { - rc = ble_l2cap_sig_disconnect(chan); - } - } - } - ble_l2cap_sig_coc_connect_cb(proc, rc); ble_l2cap_sig_proc_free(proc); diff --git a/nimble/host/src/ble_sm.c b/nimble/host/src/ble_sm.c index 2d1a4e6bb..cffec7866 100644 --- a/nimble/host/src/ble_sm.c +++ b/nimble/host/src/ble_sm.c @@ -2113,8 +2113,11 @@ ble_sm_pair_req_rx(uint16_t conn_handle, struct os_mbuf **om, proc->pair_req[0] = BLE_SM_OP_PAIR_REQ; memcpy(proc->pair_req + 1, req, sizeof(*req)); - conn = ble_hs_conn_find_assert(proc->conn_handle); - if (conn->bhc_flags & BLE_HS_CONN_F_MASTER) { + conn = ble_hs_conn_find(proc->conn_handle); + if (!conn) { + res->sm_err = BLE_SM_ERR_UNSPECIFIED; + res->app_status = BLE_HS_ENOTCONN; + } else if (conn->bhc_flags & BLE_HS_CONN_F_MASTER) { res->sm_err = BLE_SM_ERR_CMD_NOT_SUPP; res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_CMD_NOT_SUPP); } else if (ble_hs_cfg.sm_sec_lvl == 1) { diff --git a/nimble/host/src/ble_sm_sc.c b/nimble/host/src/ble_sm_sc.c index 904a6d842..183968eab 100644 --- a/nimble/host/src/ble_sm_sc.c +++ b/nimble/host/src/ble_sm_sc.c @@ -357,7 +357,13 @@ ble_sm_sc_oob_confirm(struct ble_sm_proc *proc, struct ble_sm_result *res) return; } - match = (memcmp(c, proc->oob_data_remote->c, sizeof(c)) == 0); + { + uint8_t diff = 0; + for (int j = 0; j < (int)sizeof(c); j++) { + diff |= c[j] ^ proc->oob_data_remote->c[j]; + } + match = (diff == 0); + } if (!match) { /* Random number mismatch. */ res->sm_err = BLE_SM_ERR_CONFIRM_MISMATCH; @@ -552,12 +558,18 @@ ble_sm_sc_random_rx(struct ble_sm_proc *proc, struct ble_sm_result *res) return; } - if (memcmp(proc->confirm_peer, confirm_val, 16) != 0) { - /* Random number mismatch. */ - res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_CONFIRM_MISMATCH); - res->sm_err = BLE_SM_ERR_CONFIRM_MISMATCH; - res->enc_cb = 1; - return; + { + uint8_t diff = 0; + for (int j = 0; j < 16; j++) { + diff |= proc->confirm_peer[j] ^ confirm_val[j]; + } + if (diff != 0) { + /* Random number mismatch. */ + res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_CONFIRM_MISMATCH); + res->sm_err = BLE_SM_ERR_CONFIRM_MISMATCH; + res->enc_cb = 1; + return; + } } } @@ -916,12 +928,18 @@ ble_sm_dhkey_check_process(struct ble_sm_proc *proc, return; } - if (memcmp(cmd->value, exp_value, 16) != 0) { - /* Random number mismatch. */ - res->sm_err = BLE_SM_ERR_DHKEY; - res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_DHKEY); - res->enc_cb = 1; - return; + { + uint8_t diff = 0; + for (int j = 0; j < 16; j++) { + diff |= cmd->value[j] ^ exp_value[j]; + } + if (diff != 0) { + /* Random number mismatch. */ + res->sm_err = BLE_SM_ERR_DHKEY; + res->app_status = BLE_HS_SM_US_ERR(BLE_SM_ERR_DHKEY); + res->enc_cb = 1; + return; + } } rc = ble_sm_sc_io_action(proc, &ioact);