From 50747e4f63eea4985fc07156d8675c4d124fc4c8 Mon Sep 17 00:00:00 2001 From: zhiweijian Date: Wed, 18 Mar 2026 16:34:27 +0800 Subject: [PATCH] fix(ble/bluedroid): Null/range checks, crypto cleanup and API consistency - smp_api.h/smp_int.h: SMP_OPCODE_ARRAY_SIZE and SecureConnectionOobDataReply declaration alignment - p_256_ecc_pp/p_256_multprecision: bounds and overflow fixes in ECC/multiprecision - smp_act: init le_key; p_dev_rec null check in smp_key_distribution; smp_compute_dhkey failure notify in smp_both_have_public_keys - smp_api: early state/cb_evt check in SMP_SecureConnectionOobDataReply - smp_cmac: input/length validation in cmac_aes_k_calculate and aes_cipher_msg_auth_code - smp_keys: smp_gen_p2_4_confirm return and smp_calculate_comfirm_cont; smp_process_private_key/smp_compute_dhkey cleanup and peer_pub_be clear - smp_l2c: fix callback param types with L2CAP - smp_main: event/state bounds in smp_sm_event; smp_get_event_name default string - smp_utils: cmd_code y^2 = (x^2 - 3)*x + b (mod q), so we calculate the x^2 - 3 value here */ - x_x[0] -= 3; + DWORD three[2 * KEY_LENGTH_DWORDS_P256] = {0}; + three[0] = 3; + multiprecision_sub(x_x, x_x, three, 2 * KEY_LENGTH_DWORDS_P256); /* Using math relations. (a*b) % q = ((a%q)*(b%q)) % q ==> (x^2 - 3)*x = (((x^2 - 3) % q) * x % q) % q */ multiprecision_fast_mod_P256(x_x_q, x_x); diff --git a/components/bt/host/bluedroid/stack/smp/p_256_multprecision.c b/components/bt/host/bluedroid/stack/smp/p_256_multprecision.c index 92291d98b38..d0a45a247c6 100644 --- a/components/bt/host/bluedroid/stack/smp/p_256_multprecision.c +++ b/components/bt/host/bluedroid/stack/smp/p_256_multprecision.c @@ -262,7 +262,7 @@ void multiprecision_mult(DWORD *c, DWORD *a, DWORD *b, uint32_t keyLength) DWORD V; U = V = W = 0; - multiprecision_init(c, keyLength); + multiprecision_init(c, 2 * keyLength); //assume little endian right now for (uint32_t i = 0; i < keyLength; i++) { @@ -340,7 +340,7 @@ void multiprecision_fast_mod(DWORD *c, DWORD *a) c[2] += V; V = c[2] < V; c[2] += U; - V = c[2] < U; + V += c[2] < U; c[3] += V; V = c[3] < V; c[4] += V; @@ -594,6 +594,7 @@ void multiprecision_fast_mod_P256(DWORD *c, DWORD *a) void multiprecision_inv_mod(DWORD *aminus, DWORD *u, uint32_t keyLength) { + DWORD u_local[KEY_LENGTH_DWORDS_P256]; DWORD v[KEY_LENGTH_DWORDS_P256]; DWORD A[KEY_LENGTH_DWORDS_P256 + 1]; DWORD C[KEY_LENGTH_DWORDS_P256 + 1]; @@ -605,14 +606,15 @@ void multiprecision_inv_mod(DWORD *aminus, DWORD *u, uint32_t keyLength) modp = curve.p; } + multiprecision_copy(u_local, u, keyLength); multiprecision_copy(v, modp, keyLength); multiprecision_init(A, keyLength); multiprecision_init(C, keyLength); A[0] = 1; - while (!multiprecision_iszero(u, keyLength)) { - while (!(u[0] & 0x01)) { // u is even - multiprecision_rshift(u, u, keyLength); + while (!multiprecision_iszero(u_local, keyLength)) { + while (!(u_local[0] & 0x01)) { // u is even + multiprecision_rshift(u_local, u_local, keyLength); if (!(A[0] & 0x01)) { // A is even multiprecision_rshift(A, A, keyLength); } else { @@ -633,11 +635,11 @@ void multiprecision_inv_mod(DWORD *aminus, DWORD *u, uint32_t keyLength) } } - if (multiprecision_compare(u, v, keyLength) >= 0) { - multiprecision_sub(u, u, v, keyLength); + if (multiprecision_compare(u_local, v, keyLength) >= 0) { + multiprecision_sub(u_local, u_local, v, keyLength); multiprecision_sub_mod(A, A, C, keyLength); } else { - multiprecision_sub(v, v, u, keyLength); + multiprecision_sub(v, v, u_local, keyLength); multiprecision_sub_mod(C, C, A, keyLength); } } diff --git a/components/bt/host/bluedroid/stack/smp/smp_act.c b/components/bt/host/bluedroid/stack/smp/smp_act.c index 4dbb0fa6c8a..5b3d76ddd39 100644 --- a/components/bt/host/bluedroid/stack/smp/smp_act.c +++ b/components/bt/host/bluedroid/stack/smp/smp_act.c @@ -406,7 +406,7 @@ void smp_send_id_info(tSMP_CB *p_cb, tSMP_INT_DATA *p_data) smp_send_cmd(SMP_OPCODE_ID_ADDR, p_cb); #if (BLE_INCLUDED == TRUE) - tBTM_LE_KEY_VALUE le_key; + tBTM_LE_KEY_VALUE le_key = {0}; if ((p_cb->peer_auth_req & SMP_AUTH_BOND) && (p_cb->loc_auth_req & SMP_AUTH_BOND)) { btm_sec_save_le_key(p_cb->pairing_bda, BTM_LE_KEY_LID, &le_key, TRUE); @@ -1420,7 +1420,9 @@ void smp_key_distribution(tSMP_CB *p_cb, tSMP_INT_DATA *p_data) if (smp_get_state() == SMP_STATE_BOND_PENDING) { if (p_cb->derive_lk) { tBTM_SEC_DEV_REC* p_dev_rec = btm_find_dev(p_cb->pairing_bda); - if (!(p_dev_rec->sec_flags & BTM_SEC_LE_LINK_KEY_AUTHED) && + if (p_dev_rec == NULL) { + SMP_TRACE_WARNING("%s device record not found, skip LK derivation", __func__); + } else if (!(p_dev_rec->sec_flags & BTM_SEC_LE_LINK_KEY_AUTHED) && (p_dev_rec->sec_flags & BTM_SEC_LINK_KEY_AUTHED)) { SMP_TRACE_DEBUG("%s BREDR key is higher security than existing LE keys, " "don't derive LK from LTK", __func__); @@ -1678,7 +1680,11 @@ void smp_both_have_public_keys(tSMP_CB *p_cb, tSMP_INT_DATA *p_data) SMP_TRACE_DEBUG("%s\n", __func__); /* invokes DHKey computation */ - smp_compute_dhkey(p_cb); + if (!smp_compute_dhkey(p_cb)) { + UINT8 reason = SMP_PAIR_INTERNAL_ERR; + smp_sm_event(p_cb, SMP_AUTH_CMPL_EVT, &reason); + return; + } /* on slave side invokes sending local public key to the peer */ if (p_cb->role == HCI_ROLE_SLAVE) { diff --git a/components/bt/host/bluedroid/stack/smp/smp_api.c b/components/bt/host/bluedroid/stack/smp/smp_api.c index 0b00bf151c0..7a592b775d6 100644 --- a/components/bt/host/bluedroid/stack/smp/smp_api.c +++ b/components/bt/host/bluedroid/stack/smp/smp_api.c @@ -483,6 +483,10 @@ void SMP_SecureConnectionOobDataReply(UINT8 *p_data) return; } + if (p_cb->state != SMP_STATE_WAIT_APP_RSP || p_cb->cb_evt != SMP_SC_OOB_REQ_EVT) { + return; + } + /* Set local oob data when req_oob_type = SMP_OOB_BOTH */ memcpy(&p_oob->loc_oob_data, smp_get_local_oob_data(), sizeof(tSMP_LOC_OOB_DATA)); @@ -491,10 +495,6 @@ void SMP_SecureConnectionOobDataReply(UINT8 *p_data) __FUNCTION__, p_cb->req_oob_type, p_oob->loc_oob_data.present, p_oob->peer_oob_data.present); - if (p_cb->state != SMP_STATE_WAIT_APP_RSP || p_cb->cb_evt != SMP_SC_OOB_REQ_EVT) { - return; - } - BOOLEAN data_missing = FALSE; switch (p_cb->req_oob_type) { case SMP_OOB_PEER: diff --git a/components/bt/host/bluedroid/stack/smp/smp_cmac.c b/components/bt/host/bluedroid/stack/smp/smp_cmac.c index fd10e50760e..d060dd70330 100644 --- a/components/bt/host/bluedroid/stack/smp/smp_cmac.c +++ b/components/bt/host/bluedroid/stack/smp/smp_cmac.c @@ -153,6 +153,11 @@ static BOOLEAN cmac_aes_k_calculate(BT_OCTET16 key, UINT8 *p_signature, UINT16 t SMP_TRACE_EVENT ("cmac_aes_k_calculate "); + if (cmac_cb.round == 0) { + SMP_TRACE_ERROR("%s round is 0", __func__); + return FALSE; + } + while (i <= cmac_cb.round) { smp_xor_128(&cmac_cb.text[(cmac_cb.round - i)*BT_OCTET16_LEN], x); /* Mi' := Mi (+) X */ @@ -304,6 +309,11 @@ BOOLEAN aes_cipher_msg_auth_code(BT_OCTET16 key, UINT8 *input, UINT16 length, SMP_TRACE_EVENT ("%s", __func__); + if (tlen == 0 || tlen > BT_OCTET16_LEN) { + SMP_TRACE_ERROR("%s invalid tlen=%d", __func__, tlen); + return FALSE; + } + #if (SMP_CRYPTO_MBEDTLS == TRUE) { /* diff --git a/components/bt/host/bluedroid/stack/smp/smp_keys.c b/components/bt/host/bluedroid/stack/smp/smp_keys.c index 30c602b4b1c..0ed04c0b339 100644 --- a/components/bt/host/bluedroid/stack/smp/smp_keys.c +++ b/components/bt/host/bluedroid/stack/smp/smp_keys.c @@ -672,10 +672,10 @@ BOOLEAN smp_gen_p1_4_confirm( tSMP_CB *p_cb, BT_OCTET16 p1) ** Description Generate Confirm/Compare Step2: ** p2 = padding || ia || ra ** -** Returns void +** Returns FALSE if remote address unavailable, TRUE otherwise ** *******************************************************************************/ -void smp_gen_p2_4_confirm( tSMP_CB *p_cb, BT_OCTET16 p2) +BOOLEAN smp_gen_p2_4_confirm( tSMP_CB *p_cb, BT_OCTET16 p2) { UINT8 *p = (UINT8 *)p2; BD_ADDR remote_bda; @@ -683,7 +683,7 @@ void smp_gen_p2_4_confirm( tSMP_CB *p_cb, BT_OCTET16 p2) SMP_TRACE_DEBUG ("smp_gen_p2_4_confirm\n"); if (!BTM_ReadRemoteConnectionAddr(p_cb->pairing_bda, remote_bda, &addr_type)) { SMP_TRACE_ERROR("can not generate confirm p2 for unknown device\n"); - return; + return FALSE; } SMP_TRACE_DEBUG ("smp_gen_p2_4_confirm\n"); @@ -705,6 +705,7 @@ void smp_gen_p2_4_confirm( tSMP_CB *p_cb, BT_OCTET16 p2) SMP_TRACE_DEBUG("p2 = padding || ia || ra"); smp_debug_print_nbyte_little_endian(p2, (const UINT8 *)"p2", 16); #endif + return TRUE; } /******************************************************************************* @@ -768,7 +769,10 @@ static void smp_calculate_comfirm_cont(tSMP_CB *p_cb, tSMP_ENC *p) smp_debug_print_nbyte_little_endian (p->param_buf, (const UINT8 *)"C1", 16); #endif - smp_gen_p2_4_confirm(p_cb, p2); + if (!smp_gen_p2_4_confirm(p_cb, p2)) { + smp_sm_event(p_cb, SMP_AUTH_CMPL_EVT, &status); + return; + } /* calculate p2 = (p1' XOR p2) */ smp_xor_128(p2, p->param_buf); @@ -1217,6 +1221,7 @@ void smp_process_private_key(tSMP_CB *p_cb) UINT8 priv_be[BT_OCTET32_LEN]; UINT8 pub_be[BT_OCTET32_LEN + BT_OCTET32_LEN + 1]; /* 0x04 || X (32 bytes) || Y (32 bytes) */ size_t pub_len = 0; + BOOLEAN psa_ok = FALSE; /* Convert private key from little-endian to big-endian */ for (int i = 0; i < BT_OCTET32_LEN; i++) { @@ -1250,11 +1255,16 @@ void smp_process_private_key(tSMP_CB *p_cb) p_cb->loc_publ_key.x[i] = pub_be[1 + BT_OCTET32_LEN - 1 - i]; p_cb->loc_publ_key.y[i] = pub_be[33 + BT_OCTET32_LEN - 1 - i]; } + psa_ok = TRUE; psa_pubkey_cleanup: psa_destroy_key(key_id); /* Clear sensitive data from stack */ memset(priv_be, 0, sizeof(priv_be)); + memset(pub_be, 0, sizeof(pub_be)); + if (!psa_ok) { + return; + } #elif (SMP_CRYPTO_TINYCRYPT == TRUE) { UINT8 pub_key[64]; /* TinyCrypt format: X (32 bytes) || Y (32 bytes), no prefix */ @@ -1315,10 +1325,10 @@ psa_pubkey_cleanup: ** key and peer public key; ** - saves the new public key x-coordinate as DHKey. ** -** Returns void +** Returns TRUE if DHKey was computed successfully, FALSE otherwise. ** *******************************************************************************/ -void smp_compute_dhkey (tSMP_CB *p_cb) +BOOLEAN smp_compute_dhkey (tSMP_CB *p_cb) { SMP_TRACE_DEBUG ("%s\n", __FUNCTION__); @@ -1330,6 +1340,7 @@ void smp_compute_dhkey (tSMP_CB *p_cb) UINT8 peer_pub_be[BT_OCTET32_LEN + BT_OCTET32_LEN + 1]; /* 0x04 || X (32 bytes) || Y (32 bytes) */ UINT8 shared_secret[BT_OCTET32_LEN]; size_t output_len = 0; + BOOLEAN psa_ok = FALSE; /* Convert private key from little-endian to big-endian */ for (int i = 0; i < BT_OCTET32_LEN; i++) { @@ -1369,12 +1380,17 @@ void smp_compute_dhkey (tSMP_CB *p_cb) for (int i = 0; i < BT_OCTET32_LEN; i++) { p_cb->dhkey[i] = shared_secret[BT_OCTET32_LEN - 1 - i]; } + psa_ok = TRUE; psa_dhkey_cleanup: psa_destroy_key(key_id); /* Clear sensitive data from stack */ memset(priv_be, 0, sizeof(priv_be)); + memset(peer_pub_be, 0, sizeof(peer_pub_be)); memset(shared_secret, 0, sizeof(shared_secret)); + if (!psa_ok) { + return FALSE; + } #elif (SMP_CRYPTO_TINYCRYPT == TRUE) { UINT8 priv_be[BT_OCTET32_LEN]; @@ -1395,11 +1411,11 @@ psa_dhkey_cleanup: /* Validate peer public key */ /* uECC_valid_public_key returns 0 if valid, negative value if invalid */ - if (uECC_valid_public_key(peer_pub_be, uECC_secp256r1()) < 0) { + if (uECC_valid_public_key(peer_pub_be, uECC_secp256r1()) != 0) { SMP_TRACE_ERROR("%s Invalid peer public key\n", __FUNCTION__); memset(priv_be, 0, sizeof(priv_be)); memset(peer_pub_be, 0, sizeof(peer_pub_be)); - return; + return FALSE; } /* Compute ECDH shared secret */ @@ -1409,7 +1425,7 @@ psa_dhkey_cleanup: memset(priv_be, 0, sizeof(priv_be)); memset(peer_pub_be, 0, sizeof(peer_pub_be)); memset(shared_secret, 0, sizeof(shared_secret)); - return; + return FALSE; } /* Convert shared secret from big-endian to little-endian for DHKey */ @@ -1430,9 +1446,16 @@ psa_dhkey_cleanup: memcpy(peer_publ_key.x, p_cb->peer_publ_key.x, BT_OCTET32_LEN); memcpy(peer_publ_key.y, p_cb->peer_publ_key.y, BT_OCTET32_LEN); + if (!ECC_CheckPointIsInElliCur_P256(&peer_publ_key)) { + SMP_TRACE_ERROR("%s Invalid peer public key\n", __FUNCTION__); + memset(private_key, 0, sizeof(private_key)); + return FALSE; + } + ECC_PointMult(&new_publ_key, &peer_publ_key, (DWORD *) private_key, KEY_LENGTH_DWORDS_P256); memcpy(p_cb->dhkey, new_publ_key.x, BT_OCTET32_LEN); + memset(private_key, 0, sizeof(private_key)); #endif /* SMP_CRYPTO_MBEDTLS */ smp_debug_print_nbyte_little_endian (p_cb->dhkey, (const UINT8 *)"DHKey", @@ -1444,6 +1467,7 @@ psa_dhkey_cleanup: BT_OCTET32_LEN); smp_debug_print_nbyte_little_endian (p_cb->peer_publ_key.y, (const UINT8 *)"rem public(y)", BT_OCTET32_LEN); + return TRUE; } /******************************************************************************* diff --git a/components/bt/host/bluedroid/stack/smp/smp_l2c.c b/components/bt/host/bluedroid/stack/smp/smp_l2c.c index c109b9d002c..9b019ae73fa 100644 --- a/components/bt/host/bluedroid/stack/smp/smp_l2c.c +++ b/components/bt/host/bluedroid/stack/smp/smp_l2c.c @@ -118,8 +118,8 @@ static void smp_connect_callback (UINT16 channel, BD_ADDR bd_addr, BOOLEAN conne if (memcmp(bd_addr, p_cb->pairing_bda, BD_ADDR_LEN) == 0) { SMP_TRACE_EVENT ("%s() for pairing BDA: %08x%04x Event: %s\n", __FUNCTION__, - (bd_addr[0] << 24) + (bd_addr[1] << 16) + (bd_addr[2] << 8) + bd_addr[3], - (bd_addr[4] << 8) + bd_addr[5], + ((UINT32)bd_addr[0] << 24) + ((UINT32)bd_addr[1] << 16) + ((UINT32)bd_addr[2] << 8) + bd_addr[3], + ((UINT32)bd_addr[4] << 8) + bd_addr[5], (connected) ? "connected" : "disconnected"); if (connected) { @@ -283,8 +283,8 @@ static void smp_br_connect_callback(UINT16 channel, BD_ADDR bd_addr, BOOLEAN con SMP_TRACE_EVENT ("%s for pairing BDA: %08x%04x Event: %s\n", __func__, - (bd_addr[0] << 24) + (bd_addr[1] << 16) + (bd_addr[2] << 8) + bd_addr[3], - (bd_addr[4] << 8) + bd_addr[5], + ((UINT32)bd_addr[0] << 24) + ((UINT32)bd_addr[1] << 16) + ((UINT32)bd_addr[2] << 8) + bd_addr[3], + ((UINT32)bd_addr[4] << 8) + bd_addr[5], (connected) ? "connected" : "disconnected"); if (connected) { diff --git a/components/bt/host/bluedroid/stack/smp/smp_main.c b/components/bt/host/bluedroid/stack/smp/smp_main.c index fa4d5ffbe32..fd9bc0c6d7e 100644 --- a/components/bt/host/bluedroid/stack/smp/smp_main.c +++ b/components/bt/host/bluedroid/stack/smp/smp_main.c @@ -746,7 +746,7 @@ void smp_sm_event(tSMP_CB *p_cb, tSMP_EVENT event, void *p_data) /* lookup entry /w event & curr_state */ /* If entry is ignore, return. * Otherwise, get state table (according to curr_state or all_state) */ - if ((event <= SMP_MAX_EVT) && ( (entry = entry_table[event - 1][curr_state]) != SMP_SM_IGNORE )) { + if ((event >= 1 && event <= SMP_MAX_EVT) && ( (entry = entry_table[event - 1][curr_state]) != SMP_SM_IGNORE )) { if (entry & SMP_ALL_TBL_MASK) { entry &= ~SMP_ALL_TBL_MASK; state_table = smp_all_table; @@ -803,7 +803,7 @@ const char *smp_get_event_name(tSMP_EVENT event) { const char *p_str = smp_event_name[SMP_MAX_EVT]; - if (event <= SMP_MAX_EVT) { + if (event >= 1 && event <= SMP_MAX_EVT) { p_str = smp_event_name[event - 1]; } return p_str; diff --git a/components/bt/host/bluedroid/stack/smp/smp_utils.c b/components/bt/host/bluedroid/stack/smp/smp_utils.c index 1660d78fb13..0276f8b7e2b 100644 --- a/components/bt/host/bluedroid/stack/smp/smp_utils.c +++ b/components/bt/host/bluedroid/stack/smp/smp_utils.c @@ -351,7 +351,7 @@ BOOLEAN smp_send_cmd(UINT8 cmd_code, tSMP_CB *p_cb) BOOLEAN sent = FALSE; UINT8 failure = SMP_PAIR_INTERNAL_ERR; SMP_TRACE_EVENT("smp_send_cmd on l2cap cmd_code=0x%x\n", cmd_code); - if ( cmd_code <= (SMP_OPCODE_MAX + 1 /* for SMP_OPCODE_PAIR_COMMITM */) && + if ( cmd_code < SMP_OPCODE_ARRAY_SIZE && smp_cmd_build_act[cmd_code] != NULL) { p_buf = (*smp_cmd_build_act[cmd_code])(cmd_code, p_cb); @@ -861,6 +861,11 @@ void smp_convert_string_to_tk(BT_OCTET16 tk, UINT32 passkey) void smp_mask_enc_key(UINT8 loc_enc_size, UINT8 *p_data) { SMP_TRACE_EVENT("smp_mask_enc_key\n"); + if (loc_enc_size < SMP_ENCR_KEY_SIZE_MIN) { + SMP_TRACE_ERROR("smp_mask_enc_key: loc_enc_size %d below minimum %d\n", + loc_enc_size, SMP_ENCR_KEY_SIZE_MIN); + return; + } if (loc_enc_size < BT_OCTET16_LEN) { for (; loc_enc_size < BT_OCTET16_LEN; loc_enc_size ++) { * (p_data + loc_enc_size) = 0; @@ -1060,7 +1065,7 @@ BOOLEAN smp_command_has_invalid_parameters(tSMP_CB *p_cb) SMP_TRACE_DEBUG("%s for cmd code 0x%02x\n", __func__, cmd_code); - if ((cmd_code > (SMP_OPCODE_MAX + 1 /* for SMP_OPCODE_PAIR_COMMITM */)) || + if ((cmd_code >= SMP_OPCODE_ARRAY_SIZE) || (cmd_code < SMP_OPCODE_MIN)) { SMP_TRACE_WARNING("Somehow received command with the RESERVED code 0x%02x\n", cmd_code); return TRUE;