diff --git a/components/protocomm/src/crypto/srp6a/esp_srp.c b/components/protocomm/src/crypto/srp6a/esp_srp.c index 290ec330219..fb1f0d97c2d 100644 --- a/components/protocomm/src/crypto/srp6a/esp_srp.c +++ b/components/protocomm/src/crypto/srp6a/esp_srp.c @@ -636,6 +636,7 @@ esp_err_t esp_srp_get_session_key(esp_srp_handle_t *hd, char *bytes_A, int len_A char *bytes_S; int len_S; + psa_hash_operation_t hash_op = PSA_HASH_OPERATION_INIT; u = vu = avu = S = NULL; bytes_S = NULL; @@ -677,6 +678,11 @@ esp_err_t esp_srp_get_session_key(esp_srp_handle_t *hd, char *bytes_A, int len_A if (! u) { goto error; } + if (esp_mpi_cmp_int(u, 0) == 0) { + ESP_LOGE(TAG, "Rejected SRP scrambling parameter: u == 0 (RFC 5054 Section 2.5.3)"); + ret = ESP_ERR_INVALID_ARG; + goto error; + } hexdump_mpi("u", u); /* S = (A v^u)^b */ @@ -698,7 +704,6 @@ esp_err_t esp_srp_get_session_key(esp_srp_handle_t *hd, char *bytes_A, int len_A goto error; } - psa_hash_operation_t hash_op = PSA_HASH_OPERATION_INIT; psa_status_t status = psa_hash_setup(&hash_op, PSA_ALG_SHA_512); ESP_GOTO_ON_FALSE(status == PSA_SUCCESS, ESP_FAIL, error, TAG, "Failed to setup hash operation: %d", status); psa_hash_update(&hash_op, (unsigned char *)bytes_S, len_S); diff --git a/components/protocomm/src/simple_ble/simple_ble.c b/components/protocomm/src/simple_ble/simple_ble.c index 543a4a1cde1..b1a9fdcbfd6 100644 --- a/components/protocomm/src/simple_ble/simple_ble.c +++ b/components/protocomm/src/simple_ble/simple_ble.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -43,8 +43,15 @@ const uint8_t *simple_ble_get_uuid128(uint16_t handle) { const uint8_t *uuid128_ptr; + if (g_ble_cfg_p == NULL || g_gatt_table_map == NULL) { + return NULL; + } + for (int i = 0; i < g_ble_max_gatt_table_size; i++) { if (g_gatt_table_map[i] == handle) { + if (g_ble_cfg_p->gatt_db[i].att_desc.uuid_length != ESP_UUID_LEN_128) { + return NULL; + } uuid128_ptr = (const uint8_t *) g_ble_cfg_p->gatt_db[i].att_desc.uuid_p; return uuid128_ptr; } @@ -52,16 +59,31 @@ const uint8_t *simple_ble_get_uuid128(uint16_t handle) return NULL; } +static void simple_ble_set_random_addr_if_configured(void) +{ + if (g_ble_cfg_p->ble_addr == NULL) { + return; + } + + esp_err_t err = esp_ble_gap_set_rand_addr(g_ble_cfg_p->ble_addr); + if (err == ESP_OK) { + g_ble_cfg_p->adv_params.own_addr_type = BLE_ADDR_TYPE_RANDOM; + } else { + ESP_LOGW(TAG, "Failed to set random address, using configured address type"); + } +} + static void gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) { + if (g_ble_cfg_p == NULL) { + return; + } + switch (event) { case ESP_GAP_BLE_ADV_DATA_SET_COMPLETE_EVT: adv_config_done &= (~adv_config_flag); - if (g_ble_cfg_p->ble_addr) { - esp_ble_gap_set_rand_addr(g_ble_cfg_p->ble_addr); - g_ble_cfg_p->adv_params.own_addr_type = BLE_ADDR_TYPE_RANDOM; - } + simple_ble_set_random_addr_if_configured(); if (adv_config_done == 0) { esp_ble_gap_start_advertising(&g_ble_cfg_p->adv_params); @@ -70,10 +92,7 @@ static void gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param case ESP_GAP_BLE_SCAN_RSP_DATA_SET_COMPLETE_EVT: adv_config_done &= (~scan_rsp_config_flag); - if (g_ble_cfg_p->ble_addr) { - esp_ble_gap_set_rand_addr(g_ble_cfg_p->ble_addr); - g_ble_cfg_p->adv_params.own_addr_type = BLE_ADDR_TYPE_RANDOM; - } + simple_ble_set_random_addr_if_configured(); if (adv_config_done == 0) { esp_ble_gap_start_advertising(&g_ble_cfg_p->adv_params); @@ -96,7 +115,7 @@ static void gatts_profile_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_ if (param->reg.status == ESP_GATT_OK) { gatts_if = p_gatts_if; } else { - ESP_LOGE(TAG, "reg app failed, app_id 0x0x%x, status %d", + ESP_LOGE(TAG, "reg app failed, app_id 0x%x, status %d", param->reg.app_id, param->reg.status); return; @@ -107,8 +126,15 @@ static void gatts_profile_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_ return; } + if (g_ble_cfg_p == NULL) { + return; + } + switch (event) { case ESP_GATTS_REG_EVT: + if (g_ble_cfg_p == NULL) { + return; + } ret = esp_ble_gatts_create_attr_tab(g_ble_cfg_p->gatt_db, gatts_if, g_ble_cfg_p->gatt_db_count, service_instance_id); if (ret) { ESP_LOGE(TAG, "create attr table failed, error code = 0x%x", ret); @@ -133,17 +159,23 @@ static void gatts_profile_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_ adv_config_done |= scan_rsp_config_flag; break; case ESP_GATTS_READ_EVT: - g_ble_cfg_p->read_fn(event, gatts_if, param); + if (g_ble_cfg_p) { + g_ble_cfg_p->read_fn(event, gatts_if, param); + } break; case ESP_GATTS_WRITE_EVT: - g_ble_cfg_p->write_fn(event, gatts_if, param); + if (g_ble_cfg_p) { + g_ble_cfg_p->write_fn(event, gatts_if, param); + } break; case ESP_GATTS_EXEC_WRITE_EVT: - g_ble_cfg_p->exec_write_fn(event, gatts_if, param); + if (g_ble_cfg_p) { + g_ble_cfg_p->exec_write_fn(event, gatts_if, param); + } break; case ESP_GATTS_MTU_EVT: ESP_LOGD(TAG, "ESP_GATTS_MTU_EVT, MTU %d", param->mtu.mtu); - if (g_ble_cfg_p->set_mtu_fn) { + if (g_ble_cfg_p && g_ble_cfg_p->set_mtu_fn) { g_ble_cfg_p->set_mtu_fn(event, gatts_if, param); } break; @@ -155,7 +187,9 @@ static void gatts_profile_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_ break; case ESP_GATTS_CONNECT_EVT: ESP_LOGD(TAG, "ESP_GATTS_CONNECT_EVT, conn_id = %d", param->connect.conn_id); - g_ble_cfg_p->connect_fn(event, gatts_if, param); + if (g_ble_cfg_p) { + g_ble_cfg_p->connect_fn(event, gatts_if, param); + } esp_ble_conn_update_params_t conn_params = {0}; memcpy(conn_params.bda, param->connect.remote_bda, sizeof(esp_bd_addr_t)); memcpy(s_cached_remote_bda, param->connect.remote_bda, sizeof(esp_bd_addr_t)); @@ -168,17 +202,26 @@ static void gatts_profile_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_ break; case ESP_GATTS_DISCONNECT_EVT: ESP_LOGD(TAG, "ESP_GATTS_DISCONNECT_EVT, reason = %d", param->disconnect.reason); - g_ble_cfg_p->disconnect_fn(event, gatts_if, param); + if (g_ble_cfg_p) { + g_ble_cfg_p->disconnect_fn(event, gatts_if, param); + } memset(s_cached_remote_bda, 0, sizeof(esp_bd_addr_t)); - esp_ble_gap_start_advertising(&g_ble_cfg_p->adv_params); + if (g_ble_cfg_p) { + esp_ble_gap_start_advertising(&g_ble_cfg_p->adv_params); + } break; case ESP_GATTS_CREAT_ATTR_TAB_EVT: { if (param->add_attr_tab.status != ESP_GATT_OK) { ESP_LOGE(TAG, "creating the attribute table failed, error code=0x%x", param->add_attr_tab.status); + } else if (g_ble_cfg_p == NULL) { + ESP_LOGE(TAG, "BLE config unavailable for attribute table event"); } else if (param->add_attr_tab.num_handle != g_ble_cfg_p->gatt_db_count) { ESP_LOGE(TAG, "created attribute table abnormally "); } else { ESP_LOGD(TAG, "created attribute table successfully, the number handle = %d", param->add_attr_tab.num_handle); + free(g_gatt_table_map); + g_gatt_table_map = NULL; + g_ble_max_gatt_table_size = 0; g_gatt_table_map = (uint16_t *) calloc(param->add_attr_tab.num_handle, sizeof(uint16_t)); if (g_gatt_table_map == NULL) { ESP_LOGE(TAG, "Memory allocation for GATT_TABLE_MAP failed "); @@ -217,10 +260,13 @@ simple_ble_cfg_t *simple_ble_init(void) esp_err_t simple_ble_deinit(void) { - free(g_ble_cfg_p->gatt_db); - g_ble_cfg_p->gatt_db = NULL; - free(g_ble_cfg_p); + simple_ble_cfg_t *ble_cfg = g_ble_cfg_p; g_ble_cfg_p = NULL; + if (ble_cfg) { + free(ble_cfg->gatt_db); + ble_cfg->gatt_db = NULL; + free(ble_cfg); + } free(g_gatt_table_map); g_gatt_table_map = NULL; @@ -246,7 +292,8 @@ esp_err_t simple_ble_start(simple_ble_cfg_t *cfg) #ifdef CONFIG_BTDM_CTRL_MODE_BR_EDR_ONLY ESP_LOGE(TAG, "Configuration mismatch. Select BLE Only or BTDM mode from menuconfig"); - return ESP_FAIL; + ret = ESP_FAIL; + goto err_bt_deinit; #elif CONFIG_BTDM_CTRL_MODE_BTDM ret = esp_bt_controller_enable(ESP_BT_MODE_BTDM); #else //For all other chips supporting BLE Only @@ -255,7 +302,7 @@ esp_err_t simple_ble_start(simple_ble_cfg_t *cfg) if (ret) { ESP_LOGE(TAG, "%s enable controller failed %d", __func__, ret); - return ret; + goto err_bt_deinit; } #endif @@ -263,37 +310,38 @@ esp_err_t simple_ble_start(simple_ble_cfg_t *cfg) ret = esp_bluedroid_init_with_cfg(&bluedroid_cfg); if (ret) { ESP_LOGE(TAG, "%s init bluetooth failed %d", __func__, ret); - return ret; + goto err_bt_disable; } ret = esp_bluedroid_enable(); if (ret) { ESP_LOGE(TAG, "%s enable bluetooth failed %d", __func__, ret); - return ret; + goto err_bluedroid_deinit; } - ret = esp_ble_gatts_register_callback(gatts_profile_event_handler); if (ret) { ESP_LOGE(TAG, "gatts register error, error code = 0x%x", ret); - return ret; + goto err_bluedroid_disable; } ret = esp_ble_gap_register_callback(gap_event_handler); if (ret) { ESP_LOGE(TAG, "gap register error, error code = 0x%x", ret); - return ret; + goto err_bluedroid_disable; } uint16_t app_id = 0x55; ret = esp_ble_gatts_app_register(app_id); if (ret) { ESP_LOGE(TAG, "gatts app register error, error code = 0x%x", ret); - return ret; + goto err_bluedroid_disable; } esp_err_t local_mtu_ret = esp_ble_gatt_set_local_mtu(500); if (local_mtu_ret) { ESP_LOGE(TAG, "set local MTU failed, error code = 0x%x", local_mtu_ret); + ret = local_mtu_ret; + goto err_bluedroid_disable; } ESP_LOGD(TAG, "Free mem at end of simple_ble_init %" PRIu32, esp_get_free_heap_size()); @@ -317,6 +365,18 @@ esp_err_t simple_ble_start(simple_ble_cfg_t *cfg) esp_ble_gap_set_security_param(ESP_BLE_SM_SET_RSP_KEY, &rsp_key, sizeof(uint8_t)); return ESP_OK; + +err_bluedroid_disable: + esp_bluedroid_disable(); +err_bluedroid_deinit: + esp_bluedroid_deinit(); +err_bt_disable: +#ifdef CONFIG_BT_CONTROLLER_ENABLED + esp_bt_controller_disable(); +err_bt_deinit: + esp_bt_controller_deinit(); +#endif + return ret; } esp_err_t simple_ble_stop(void) @@ -357,3 +417,15 @@ esp_err_t simple_ble_disconnect(void) { return esp_ble_gap_disconnect(s_cached_remote_bda); } + +void simple_ble_gatts_clear_char_values(void) +{ + if (g_ble_cfg_p == NULL || g_gatt_table_map == NULL) { + return; + } + for (int i = 0; i < g_ble_max_gatt_table_size; i++) { + if (g_ble_cfg_p->gatt_db[i].att_desc.uuid_length == ESP_UUID_LEN_128) { + esp_ble_gatts_set_attr_value(g_gatt_table_map[i], 0, NULL); + } + } +} diff --git a/components/protocomm/src/simple_ble/simple_ble.h b/components/protocomm/src/simple_ble/simple_ble.h index dd3858096fa..e38290b6d3b 100644 --- a/components/protocomm/src/simple_ble/simple_ble.h +++ b/components/protocomm/src/simple_ble/simple_ble.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -126,4 +126,12 @@ const uint8_t *simple_ble_get_uuid128(uint16_t handle); * @return ESP_OK on success, and appropriate error code for failure */ esp_err_t simple_ble_disconnect(void); + +/** Clear all characteristic value attributes in the GATT table + * + * Resets the stored value of every 128-bit-UUID characteristic (i.e. every + * response written via esp_ble_gatts_set_attr_value) to zero length so that + * a new connection cannot read the previous session's response data. + */ +void simple_ble_gatts_clear_char_values(void); #endif /* _SIMPLE_BLE_ */ diff --git a/components/protocomm/src/transports/protocomm_ble.c b/components/protocomm/src/transports/protocomm_ble.c index 87ef813ae13..a64fd1d84a8 100644 --- a/components/protocomm/src/transports/protocomm_ble.c +++ b/components/protocomm/src/transports/protocomm_ble.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2018-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2018-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -37,9 +38,11 @@ static const char *TAG = "protocomm_ble"; static const uint16_t primary_service_uuid = ESP_GATT_UUID_PRI_SERVICE; static const uint16_t character_declaration_uuid = ESP_GATT_UUID_CHAR_DECLARE; static const uint16_t character_user_description = ESP_GATT_UUID_CHAR_DESCRIPTION; +static const uint16_t character_client_config_uuid = ESP_GATT_UUID_CHAR_CLIENT_CONFIG; static const uint8_t character_prop_read_write = ESP_GATT_CHAR_PROP_BIT_READ | ESP_GATT_CHAR_PROP_BIT_WRITE; static const uint8_t character_prop_read_write_notify = ESP_GATT_CHAR_PROP_BIT_READ | ESP_GATT_CHAR_PROP_BIT_WRITE | \ ESP_GATT_CHAR_PROP_BIT_NOTIFY; +static const uint8_t character_cccd_value[2] = {0x00, 0x00}; typedef struct { uint8_t type; @@ -63,7 +66,7 @@ typedef struct name_uuid128 { typedef struct _protocomm_ble { protocomm_t *pc_ble; name_uuid128_t *g_nu_lookup; - ssize_t g_nu_lookup_count; + size_t g_nu_lookup_count; uint16_t gatt_mtu; uint8_t *service_uuid; unsigned ble_link_encryption:1; @@ -130,9 +133,11 @@ static void hexdump(const char *msg, uint8_t *buf, int len) ESP_LOG_BUFFER_HEX_LEVEL(TAG, buf, len, ESP_LOG_DEBUG); } -static const uint16_t *uuid128_to_16(const uint8_t *uuid128) +static uint16_t uuid128_to_16(const uint8_t *uuid128) { - return (const uint16_t *) &uuid128[12]; + uint16_t uuid16 = 0; + memcpy(&uuid16, &uuid128[12], sizeof(uuid16)); + return uuid16; } static const char *handle_to_handler(uint16_t handle) @@ -144,8 +149,9 @@ static const char *handle_to_handler(uint16_t handle) if (!uuid128) { return NULL; } - for (int i = 0; i < protoble_internal->g_nu_lookup_count; i++) { - if (*uuid128_to_16(protoble_internal->g_nu_lookup[i].uuid128) == *uuid128_to_16(uuid128)) { + uint16_t target_uuid16 = uuid128_to_16(uuid128); + for (size_t i = 0; i < protoble_internal->g_nu_lookup_count; i++) { + if (uuid128_to_16(protoble_internal->g_nu_lookup[i].uuid128) == target_uuid16) { return protoble_internal->g_nu_lookup[i].name; } } @@ -173,7 +179,7 @@ static void transport_simple_ble_read(esp_gatts_cb_event_t event, esp_gatt_if_t ESP_LOGD(TAG, "Inside read w/ session - %d on param %d %d", param->read.conn_id, param->read.handle, read_len); - if (!read_len && !param->read.offset) { + if (!param->read.offset) { ESP_LOGD(TAG, "Reading attr value first time"); status = esp_ble_gatts_get_attr_value(param->read.handle, &read_len, &read_buf); max_read_len = read_len; @@ -234,12 +240,17 @@ static esp_err_t prepare_write_event_env(esp_gatt_if_t gatts_if, /* If prepare buffer is allocated copy incoming data into it */ if (status == ESP_GATT_OK) { - memcpy(prepare_write_env.prepare_buf + param->write.offset, - param->write.value, - param->write.len); - int next_len = param->write.offset + param->write.len; - prepare_write_env.prepare_len = MAX(prepare_write_env.prepare_len, next_len); - prepare_write_env.handle = param->write.handle; + if (param->write.len && param->write.value) { + memcpy(prepare_write_env.prepare_buf + param->write.offset, + param->write.value, + param->write.len); + int next_len = param->write.offset + param->write.len; + prepare_write_env.prepare_len = MAX(prepare_write_env.prepare_len, next_len); + prepare_write_env.handle = param->write.handle; + } else if (param->write.len) { + ESP_LOGE(TAG, "NULL write value for non-zero length"); + status = ESP_GATT_ERROR; + } } /* Send write response if needed */ @@ -297,6 +308,16 @@ static void transport_simple_ble_write(esp_gatts_cb_event_t event, esp_gatt_if_t return; } + protocomm_t *pc_ble = protoble_internal->pc_ble; + if (pc_ble == NULL) { + ESP_LOGW(TAG, "Ignoring write on inactive protocomm transport"); + if (param->write.need_rsp) { + esp_ble_gatts_send_response(gatts_if, param->write.conn_id, + param->write.trans_id, ESP_GATT_ERROR, NULL); + } + return; + } + if (param->write.is_prep) { ret = prepare_write_event_env(gatts_if, param); if (ret != ESP_OK) { @@ -307,26 +328,71 @@ static void transport_simple_ble_write(esp_gatts_cb_event_t event, esp_gatt_if_t ESP_LOGD(TAG, "is_prep not set"); } - ret = protocomm_req_handle(protoble_internal->pc_ble, - handle_to_handler(param->write.handle), + if (param->write.len == 0 || param->write.len > CHAR_VAL_LEN_MAX) { + ESP_LOGE(TAG, "Invalid write length %d for handle %d", param->write.len, param->write.handle); + if (param->write.need_rsp) { + esp_ble_gatts_send_response(gatts_if, param->write.conn_id, + param->write.trans_id, ESP_GATT_INVALID_ATTR_LEN, NULL); + } + return; + } + + const char *ep_name = handle_to_handler(param->write.handle); + if (ep_name == NULL) { + ESP_LOGW(TAG, "No endpoint mapped for handle %d", param->write.handle); + if (param->write.need_rsp) { + esp_ble_gatts_send_response(gatts_if, param->write.conn_id, + param->write.trans_id, ESP_GATT_NOT_FOUND, NULL); + } + return; + } + + ret = protocomm_req_handle(pc_ble, + ep_name, param->write.conn_id, param->write.value, param->write.len, &outbuf, &outlen); if (ret == ESP_OK) { + if (outlen < 0 || outlen > CHAR_VAL_LEN_MAX) { + ESP_LOGE(TAG, "Invalid response length %d for handle %d", (int)outlen, param->write.handle); + if (outbuf) { + free(outbuf); + } + if (param->write.need_rsp) { + esp_ble_gatts_send_response(gatts_if, param->write.conn_id, + param->write.trans_id, ESP_GATT_INVALID_ATTR_LEN, NULL); + } + return; + } + if (outlen > 0 && outbuf == NULL) { + ESP_LOGE(TAG, "NULL response buffer for non-zero response length"); + if (param->write.need_rsp) { + esp_ble_gatts_send_response(gatts_if, param->write.conn_id, + param->write.trans_id, ESP_GATT_ERROR, NULL); + } + return; + } + ret = esp_ble_gatts_set_attr_value(param->write.handle, outlen, outbuf); if (ret != ESP_OK) { ESP_LOGE(TAG, "Failed to set the session attribute value"); } - ret = esp_ble_gatts_send_response(gatts_if, param->write.conn_id, - param->write.trans_id, ESP_GATT_OK, NULL); - if (ret != ESP_OK) { - ESP_LOGE(TAG, "Send response error in write"); + if (param->write.need_rsp) { + ret = esp_ble_gatts_send_response(gatts_if, param->write.conn_id, + param->write.trans_id, ESP_GATT_OK, NULL); + if (ret != ESP_OK) { + ESP_LOGE(TAG, "Send response error in write"); + } } hexdump("Response from write", outbuf, outlen); } else { ESP_LOGE(TAG, "Invalid content received, killing connection"); + if (param->write.need_rsp) { + esp_ble_gatts_send_response(gatts_if, param->write.conn_id, + param->write.trans_id, ESP_GATT_ERROR, NULL); + } esp_ble_gatts_close(gatts_if, param->write.conn_id); } if (outbuf) { @@ -350,11 +416,39 @@ static void transport_simple_ble_exec_write(esp_gatts_cb_event_t event, esp_gatt return; } + protocomm_t *pc_ble = protoble_internal->pc_ble; + if (pc_ble == NULL) { + ESP_LOGW(TAG, "Ignoring exec write on inactive protocomm transport"); + protocomm_ble_reset_prepare_write(); + esp_ble_gatts_send_response(gatts_if, param->exec_write.conn_id, + param->exec_write.trans_id, ESP_GATT_ERROR, NULL); + esp_ble_gatts_close(gatts_if, param->exec_write.conn_id); + return; + } + if ((param->exec_write.exec_write_flag == ESP_GATT_PREP_WRITE_EXEC) && prepare_write_env.prepare_buf) { - err = protocomm_req_handle(protoble_internal->pc_ble, - handle_to_handler(prepare_write_env.handle), + if (prepare_write_env.prepare_len <= 0 || prepare_write_env.prepare_len > PREPARE_BUF_MAX_SIZE) { + ESP_LOGE(TAG, "Invalid prepared write length: %d", prepare_write_env.prepare_len); + esp_ble_gatts_send_response(gatts_if, param->exec_write.conn_id, + param->exec_write.trans_id, ESP_GATT_INVALID_ATTR_LEN, NULL); + protocomm_ble_reset_prepare_write(); + return; + } + + const char *ep_name = handle_to_handler(prepare_write_env.handle); + if (ep_name == NULL) { + ESP_LOGE(TAG, "No endpoint mapped for prepared write handle %d", prepare_write_env.handle); + esp_ble_gatts_send_response(gatts_if, param->exec_write.conn_id, + param->exec_write.trans_id, ESP_GATT_NOT_FOUND, NULL); + esp_ble_gatts_close(gatts_if, param->exec_write.conn_id); + protocomm_ble_reset_prepare_write(); + return; + } + + err = protocomm_req_handle(pc_ble, + ep_name, param->exec_write.conn_id, prepare_write_env.prepare_buf, prepare_write_env.prepare_len, @@ -362,8 +456,33 @@ static void transport_simple_ble_exec_write(esp_gatts_cb_event_t event, esp_gatt if (err != ESP_OK) { ESP_LOGE(TAG, "Invalid content received, killing connection"); + esp_ble_gatts_send_response(gatts_if, param->exec_write.conn_id, + param->exec_write.trans_id, ESP_GATT_ERROR, NULL); esp_ble_gatts_close(gatts_if, param->exec_write.conn_id); + protocomm_ble_reset_prepare_write(); + if (outbuf) { + free(outbuf); + } + return; } else { + if (outlen < 0 || outlen > CHAR_VAL_LEN_MAX) { + ESP_LOGE(TAG, "Invalid response length %d in exec write", (int)outlen); + if (outbuf) { + free(outbuf); + outbuf = NULL; + } + esp_ble_gatts_send_response(gatts_if, param->exec_write.conn_id, + param->exec_write.trans_id, ESP_GATT_INVALID_ATTR_LEN, NULL); + protocomm_ble_reset_prepare_write(); + return; + } + if (outlen > 0 && outbuf == NULL) { + ESP_LOGE(TAG, "NULL response buffer for non-zero exec write response"); + esp_ble_gatts_send_response(gatts_if, param->exec_write.conn_id, + param->exec_write.trans_id, ESP_GATT_ERROR, NULL); + protocomm_ble_reset_prepare_write(); + return; + } hexdump("Response from exec write", outbuf, outlen); esp_ble_gatts_set_attr_value(prepare_write_env.handle, outlen, outbuf); } @@ -391,6 +510,10 @@ static void transport_simple_ble_disconnect(esp_gatts_cb_event_t event, esp_gatt /* Drop any staged prepare-write data when a connection ends */ protocomm_ble_reset_prepare_write(); + /* Clear GATT attribute values so a new connection cannot read the + * previous session's response data. */ + simple_ble_gatts_clear_char_values(); + /* Ignore BLE events received after protocomm layer is stopped */ if (protoble_internal == NULL) { ESP_LOGI(TAG,"Protocomm layer has already stopped"); @@ -402,9 +525,14 @@ static void transport_simple_ble_disconnect(esp_gatts_cb_event_t event, esp_gatt return; } - if (protoble_internal->pc_ble->sec && - protoble_internal->pc_ble->sec->close_transport_session) { - ret = protoble_internal->pc_ble->sec->close_transport_session(protoble_internal->pc_ble->sec_inst, + protocomm_t *pc_ble = protoble_internal->pc_ble; + if (pc_ble == NULL) { + ESP_LOGD(TAG, "Protocomm BLE inactive, ignoring disconnect"); + return; + } + + if (pc_ble->sec && pc_ble->sec->close_transport_session) { + ret = pc_ble->sec->close_transport_session(pc_ble->sec_inst, param->disconnect.conn_id); if (ret != ESP_OK) { ESP_LOGE(TAG, "error closing the session after disconnect"); @@ -414,6 +542,7 @@ static void transport_simple_ble_disconnect(esp_gatts_cb_event_t event, esp_gatt ble_event.evt_type = PROTOCOMM_TRANSPORT_BLE_DISCONNECTED; /* Set the Disconnection handle */ ble_event.conn_handle = param->disconnect.conn_id; + ble_event.disconnect_reason = param->disconnect.reason; if (esp_event_post(PROTOCOMM_TRANSPORT_BLE_EVENT, PROTOCOMM_TRANSPORT_BLE_DISCONNECTED, &ble_event, sizeof(protocomm_ble_event_t), portMAX_DELAY) != ESP_OK) { ESP_LOGE(TAG, "Failed to post transport disconnection event"); @@ -442,9 +571,14 @@ static void transport_simple_ble_connect(esp_gatts_cb_event_t event, esp_gatt_if return; } - if (protoble_internal->pc_ble->sec && - protoble_internal->pc_ble->sec->new_transport_session) { - ret = protoble_internal->pc_ble->sec->new_transport_session(protoble_internal->pc_ble->sec_inst, + protocomm_t *pc_ble = protoble_internal->pc_ble; + if (pc_ble == NULL) { + ESP_LOGD(TAG, "Protocomm BLE inactive, ignoring connect"); + return; + } + + if (pc_ble->sec && pc_ble->sec->new_transport_session) { + ret = pc_ble->sec->new_transport_session(pc_ble->sec_inst, param->connect.conn_id); if (ret != ESP_OK) { ESP_LOGE(TAG, "error creating the session"); @@ -488,17 +622,29 @@ static esp_err_t protocomm_ble_remove_endpoint(const char *ep_name) static ssize_t populate_gatt_db(esp_gatts_attr_db_t **gatt_db_generated) { int i; - /* Each endpoint requires 3 attributes: + int char_stride = protoble_internal->ble_notify ? 4 : 3; + /* Each endpoint requires 3 (or 4 if notify enabled) attributes: * 1) for Characteristic Declaration * 2) for Characteristic Value (for reading and writing to an endpoint) * 3) for Characteristic User Description (endpoint name) + * 4) for Client Characteristic Configuration Descriptor (if notify enabled) * - * Therefore, we need esp_gatts_attr_db_t of size 3 * number of endpoints + 1 for service + * Therefore, we need esp_gatts_attr_db_t of size char_stride * number of endpoints + 1 for service */ - ssize_t gatt_db_generated_entries = 3 * protoble_internal->g_nu_lookup_count + 1; + if (protoble_internal->g_nu_lookup_count > ((SIZE_MAX - 1) / char_stride)) { + ESP_LOGE(TAG, "gatt db entries overflow"); + return -1; + } + size_t gatt_db_generated_entries_sz = char_stride * protoble_internal->g_nu_lookup_count + 1; + if (gatt_db_generated_entries_sz > (size_t)INT_MAX || + gatt_db_generated_entries_sz > (SIZE_MAX / sizeof(esp_gatts_attr_db_t))) { + ESP_LOGE(TAG, "gatt db size overflow"); + return -1; + } + ssize_t gatt_db_generated_entries = (ssize_t)gatt_db_generated_entries_sz; *gatt_db_generated = (esp_gatts_attr_db_t *) malloc(sizeof(esp_gatts_attr_db_t) * - (gatt_db_generated_entries)); + gatt_db_generated_entries_sz); if ((*gatt_db_generated) == NULL) { ESP_LOGE(TAG, "Failed to assign memory to gatt_db"); return -1; @@ -515,9 +661,12 @@ static ssize_t populate_gatt_db(esp_gatts_attr_db_t **gatt_db_generated) /* Declare characteristics */ for (i = 1 ; i < gatt_db_generated_entries ; i++) { + int attr_idx = (i - 1) % char_stride; + int ep_idx = (i - 1) / char_stride; + (*gatt_db_generated)[i].attr_control.auto_rsp = ESP_GATT_RSP_BY_APP; - if (i % 3 == 1) { + if (attr_idx == 0) { /* Characteristic Declaration */ (*gatt_db_generated)[i].att_desc.perm = ESP_GATT_PERM_READ; (*gatt_db_generated)[i].att_desc.uuid_length = ESP_UUID_LEN_16; @@ -530,25 +679,34 @@ static ssize_t populate_gatt_db(esp_gatts_attr_db_t **gatt_db_generated) } else { (*gatt_db_generated)[i].att_desc.value = (uint8_t *) &character_prop_read_write; } - } else if (i % 3 == 2) { + } else if (attr_idx == 1) { /* Characteristic Value */ (*gatt_db_generated)[i].att_desc.perm = ESP_GATT_PERM_READ | ESP_GATT_PERM_WRITE ; if (protoble_internal->ble_link_encryption) { (*gatt_db_generated)[i].att_desc.perm |= ESP_GATT_PERM_READ_ENCRYPTED | ESP_GATT_PERM_WRITE_ENCRYPTED; } (*gatt_db_generated)[i].att_desc.uuid_length = ESP_UUID_LEN_128; - (*gatt_db_generated)[i].att_desc.uuid_p = protoble_internal->g_nu_lookup[i / 3].uuid128; + (*gatt_db_generated)[i].att_desc.uuid_p = protoble_internal->g_nu_lookup[ep_idx].uuid128; (*gatt_db_generated)[i].att_desc.max_length = CHAR_VAL_LEN_MAX; (*gatt_db_generated)[i].att_desc.length = 0; (*gatt_db_generated)[i].att_desc.value = NULL; - } else { + } else if (attr_idx == 2) { /* Characteristic User Description (for keeping endpoint names) */ (*gatt_db_generated)[i].att_desc.perm = ESP_GATT_PERM_READ; (*gatt_db_generated)[i].att_desc.uuid_length = ESP_UUID_LEN_16; (*gatt_db_generated)[i].att_desc.uuid_p = (uint8_t *) &character_user_description; - (*gatt_db_generated)[i].att_desc.max_length = strlen(protoble_internal->g_nu_lookup[i / 3 - 1].name); + (*gatt_db_generated)[i].att_desc.max_length = strlen(protoble_internal->g_nu_lookup[ep_idx].name); (*gatt_db_generated)[i].att_desc.length = (*gatt_db_generated)[i].att_desc.max_length; - (*gatt_db_generated)[i].att_desc.value = (uint8_t *) protoble_internal->g_nu_lookup[i / 3 - 1].name; + (*gatt_db_generated)[i].att_desc.value = (uint8_t *) protoble_internal->g_nu_lookup[ep_idx].name; + } else { + /* Client Characteristic Configuration Descriptor */ + (*gatt_db_generated)[i].attr_control.auto_rsp = ESP_GATT_AUTO_RSP; + (*gatt_db_generated)[i].att_desc.perm = ESP_GATT_PERM_READ | ESP_GATT_PERM_WRITE; + (*gatt_db_generated)[i].att_desc.uuid_length = ESP_UUID_LEN_16; + (*gatt_db_generated)[i].att_desc.uuid_p = (uint8_t *) &character_client_config_uuid; + (*gatt_db_generated)[i].att_desc.max_length = sizeof(uint16_t); + (*gatt_db_generated)[i].att_desc.length = sizeof(uint16_t); + (*gatt_db_generated)[i].att_desc.value = (uint8_t *) character_cccd_value; } } return gatt_db_generated_entries; @@ -558,8 +716,14 @@ static void protocomm_ble_cleanup(void) { protocomm_ble_reset_prepare_write(); if (protoble_internal) { + if (protoble_internal->service_uuid) { + free(protoble_internal->service_uuid); + protoble_internal->service_uuid = NULL; + } + adv_config.p_service_uuid = NULL; + adv_config.service_uuid_len = 0; if (protoble_internal->g_nu_lookup) { - for (unsigned i = 0; i < protoble_internal->g_nu_lookup_count; i++) { + for (size_t i = 0; i < protoble_internal->g_nu_lookup_count; i++) { if (protoble_internal->g_nu_lookup[i].name) { free((void *)protoble_internal->g_nu_lookup[i].name); } @@ -578,6 +742,10 @@ static void protocomm_ble_cleanup(void) protocomm_ble_mfg_data = NULL; protocomm_ble_mfg_data_len = 0; } + if (protocomm_ble_addr) { + free(protocomm_ble_addr); + protocomm_ble_addr = NULL; + } } esp_err_t protocomm_ble_start(protocomm_t *pc, const protocomm_ble_config_t *config) @@ -586,11 +754,34 @@ esp_err_t protocomm_ble_start(protocomm_t *pc, const protocomm_ble_config_t *con return ESP_ERR_INVALID_ARG; } + if (config->manufacturer_data_len > 0 && config->manufacturer_data == NULL) { + ESP_LOGE(TAG, "Manufacturer data length set without data"); + return ESP_ERR_INVALID_ARG; + } + + if (config->nu_lookup_count <= 0 || config->nu_lookup_count > (ssize_t)(INT_MAX - 1)) { + ESP_LOGE(TAG, "Invalid nu_lookup_count: %d", (int)config->nu_lookup_count); + return ESP_ERR_INVALID_ARG; + } + + if (config->manufacturer_data != NULL && + (config->manufacturer_data_len <= 0 || + config->manufacturer_data_len > MAX_BLE_MANUFACTURER_DATA_LEN)) { + ESP_LOGE(TAG, "Invalid manufacturer data length: %d", (int)config->manufacturer_data_len); + return ESP_ERR_INVALID_ARG; + } + if (protoble_internal) { ESP_LOGE(TAG, "Protocomm BLE already started"); return ESP_FAIL; } + size_t endpoint_count = (size_t)config->nu_lookup_count; + if (endpoint_count > (SIZE_MAX / sizeof(name_uuid128_t))) { + ESP_LOGE(TAG, "Name UUID table size overflow"); + return ESP_ERR_NO_MEM; + } + /* Store BLE device name internally */ protocomm_ble_device_name = strdup(config->device_name); if (protocomm_ble_device_name == NULL) { @@ -601,12 +792,24 @@ esp_err_t protocomm_ble_start(protocomm_t *pc, const protocomm_ble_config_t *con /* Store BLE manufacturer data pointer */ if (config->manufacturer_data != NULL) { - protocomm_ble_mfg_data = config->manufacturer_data; - protocomm_ble_mfg_data_len = config->manufacturer_data_len; + protocomm_ble_mfg_data = (uint8_t *)malloc((size_t)config->manufacturer_data_len); + if (protocomm_ble_mfg_data == NULL) { + ESP_LOGE(TAG, "Error allocating memory for manufacturer data"); + protocomm_ble_cleanup(); + return ESP_ERR_NO_MEM; + } + memcpy(protocomm_ble_mfg_data, config->manufacturer_data, (size_t)config->manufacturer_data_len); + protocomm_ble_mfg_data_len = (size_t)config->manufacturer_data_len; } if (config->ble_addr != NULL) { - protocomm_ble_addr = config->ble_addr; + protocomm_ble_addr = (uint8_t *)malloc(BLE_ADDR_LEN); + if (protocomm_ble_addr == NULL) { + ESP_LOGE(TAG, "Error allocating memory for BLE address"); + protocomm_ble_cleanup(); + return ESP_ERR_NO_MEM; + } + memcpy(protocomm_ble_addr, config->ble_addr, BLE_ADDR_LEN); } protoble_internal = (_protocomm_ble_internal_t *) calloc(1, sizeof(_protocomm_ble_internal_t)); @@ -616,19 +819,25 @@ esp_err_t protocomm_ble_start(protocomm_t *pc, const protocomm_ble_config_t *con return ESP_ERR_NO_MEM; } - protoble_internal->g_nu_lookup_count = config->nu_lookup_count; - protoble_internal->g_nu_lookup = malloc(config->nu_lookup_count * sizeof(name_uuid128_t)); + protoble_internal->g_nu_lookup_count = endpoint_count; + protoble_internal->g_nu_lookup = calloc(endpoint_count, sizeof(name_uuid128_t)); if (protoble_internal->g_nu_lookup == NULL) { ESP_LOGE(TAG, "Error allocating internal name UUID table"); protocomm_ble_cleanup(); return ESP_ERR_NO_MEM; } - for (unsigned i = 0; i < protoble_internal->g_nu_lookup_count; i++) { + for (size_t i = 0; i < protoble_internal->g_nu_lookup_count; i++) { memcpy(protoble_internal->g_nu_lookup[i].uuid128, config->service_uuid, ESP_UUID_LEN_128); - memcpy((uint8_t *)uuid128_to_16(protoble_internal->g_nu_lookup[i].uuid128), + memcpy((uint8_t *)&protoble_internal->g_nu_lookup[i].uuid128[12], &config->nu_lookup[i].uuid, ESP_UUID_LEN_16); + if (config->nu_lookup[i].name == NULL) { + ESP_LOGE(TAG, "Invalid endpoint name"); + protocomm_ble_cleanup(); + return ESP_ERR_INVALID_ARG; + } + protoble_internal->g_nu_lookup[i].name = strdup(config->nu_lookup[i].name); if (protoble_internal->g_nu_lookup[i].name == NULL) { ESP_LOGE(TAG, "Error allocating internal name UUID entry"); @@ -646,8 +855,14 @@ esp_err_t protocomm_ble_start(protocomm_t *pc, const protocomm_ble_config_t *con // Config adv data adv_config.service_uuid_len = ESP_UUID_LEN_128; - adv_config.p_service_uuid = (uint8_t *) config->service_uuid; - protoble_internal->service_uuid = (uint8_t *) config->service_uuid; + protoble_internal->service_uuid = (uint8_t *)malloc(ESP_UUID_LEN_128); + if (protoble_internal->service_uuid == NULL) { + ESP_LOGE(TAG, "Error allocating memory for service UUID"); + protocomm_ble_cleanup(); + return ESP_ERR_NO_MEM; + } + memcpy(protoble_internal->service_uuid, config->service_uuid, ESP_UUID_LEN_128); + adv_config.p_service_uuid = protoble_internal->service_uuid; // Config scan response data scan_rsp_config.manufacturer_len = protocomm_ble_mfg_data_len; @@ -689,7 +904,8 @@ esp_err_t protocomm_ble_start(protocomm_t *pc, const protocomm_ble_config_t *con if (ble_config->gatt_db_count == -1) { ESP_LOGE(TAG, "Invalid GATT database count"); - simple_ble_deinit(); + free(ble_config->gatt_db); + free(ble_config); protocomm_ble_cleanup(); return ESP_ERR_INVALID_STATE; } @@ -727,6 +943,8 @@ esp_err_t protocomm_ble_stop(protocomm_t *pc) ret = simple_ble_disconnect(); if (ret) { ESP_LOGE(TAG, "BLE disconnect failed"); + protoble_internal->pc_ble = pc; + return ret; } simple_ble_deinit(); ble_callbacks_active = false; @@ -740,6 +958,8 @@ esp_err_t protocomm_ble_stop(protocomm_t *pc) ret = simple_ble_stop(); if (ret) { ESP_LOGE(TAG, "BLE stop failed"); + protoble_internal->pc_ble = pc; + return ret; } simple_ble_deinit(); ble_callbacks_active = false; diff --git a/components/protocomm/src/transports/protocomm_nimble.c b/components/protocomm/src/transports/protocomm_nimble.c index a07af70af84..2f46b6d01a5 100644 --- a/components/protocomm/src/transports/protocomm_nimble.c +++ b/components/protocomm/src/transports/protocomm_nimble.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2019-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -35,6 +36,13 @@ static uint16_t s_cached_conn_handle; /* Standard 16 bit UUID for characteristic User Description*/ #define BLE_GATT_UUID_CHAR_DSC 0x2901 +/* NimBLE ATT attribute values are bounded; enforce the same bound locally. */ +#ifndef BLE_ATT_ATTR_MAX_LEN +#define BLE_ATT_ATTR_MAX_LEN 512 +#endif + +#define PROTOCOMM_NIMBLE_MAX_PAYLOAD_LEN BLE_ATT_ATTR_MAX_LEN + /******************************************************** * Maintain database for Attribute specific data * ********************************************************/ @@ -68,7 +76,7 @@ void ble_store_config_init(void); typedef struct _protocomm_ble { protocomm_t *pc_ble; protocomm_ble_name_uuid_t *g_nu_lookup; - ssize_t g_nu_lookup_count; + size_t g_nu_lookup_count; uint16_t gatt_mtu; unsigned ble_link_encryption:1; unsigned ble_notify:1; @@ -117,6 +125,7 @@ typedef void (simple_ble_cb_t)(struct ble_gap_event *event, void *arg); static void transport_simple_ble_connect(struct ble_gap_event *event, void *arg); static void transport_simple_ble_disconnect(struct ble_gap_event *event, void *arg); static void transport_simple_ble_set_mtu(struct ble_gap_event *event, void *arg); +static void simple_ble_gatts_clear_cached_values(void); typedef struct { /** Name to be displayed to devices scanning for ESP32 */ @@ -193,6 +202,11 @@ simple_ble_advertise(void) { int rc; + if (adv_data.uuids128 == NULL) { + ESP_LOGD(TAG, "Not advertising: UUID data already freed"); + return; + } + adv_data.flags = (BLE_HS_ADV_F_DISC_GEN | BLE_HS_ADV_F_BREDR_UNSUP); adv_data.num_uuids128 = 1; adv_data.uuids128_is_complete = 1; @@ -259,9 +273,6 @@ simple_ble_gap_event(struct ble_gap_event *event, void *arg) transport_simple_ble_disconnect(event, arg); /* Clear conn_handle value */ s_cached_conn_handle = 0; - if (esp_event_post(PROTOCOMM_TRANSPORT_BLE_EVENT, PROTOCOMM_TRANSPORT_BLE_DISCONNECTED, NULL, 0, portMAX_DELAY) != ESP_OK) { - ESP_LOGE(TAG, "Failed to post pairing event"); - } /* Connection terminated; resume advertising. */ simple_ble_advertise(); return 0; @@ -297,12 +308,14 @@ static const char *uuid128_to_handler(uint8_t *uuid) } /* Use it to convert 128 bit UUID to 16 bit UUID.*/ uint8_t *uuid16 = uuid + 12; - for (int i = 0; i < protoble_internal->g_nu_lookup_count; i++) { - if (protoble_internal->g_nu_lookup[i].uuid == *(uint16_t *)uuid16 ) { - ESP_LOGD(TAG, "UUID (0x%x) matched with proto-name = %s", *uuid16, protoble_internal->g_nu_lookup[i].name); + uint16_t short_uuid = 0; + memcpy(&short_uuid, uuid16, sizeof(short_uuid)); + for (size_t i = 0; i < protoble_internal->g_nu_lookup_count; i++) { + if (protoble_internal->g_nu_lookup[i].uuid == short_uuid) { + ESP_LOGD(TAG, "UUID (0x%x) matched with proto-name = %s", short_uuid, protoble_internal->g_nu_lookup[i].name); return protoble_internal->g_nu_lookup[i].name; } else { - ESP_LOGD(TAG, "UUID did not match... %x", *uuid16); + ESP_LOGD(TAG, "UUID did not match... %x", short_uuid); } } return NULL; @@ -323,11 +336,20 @@ gatt_svr_dsc_access(uint16_t conn_handle, uint16_t attr_handle, struct return BLE_ATT_ERR_UNLIKELY; } - int rc; - ssize_t temp_outlen = strlen(ctxt->dsc->arg); + if (ctxt->dsc == NULL || ctxt->dsc->arg == NULL) { + ESP_LOGE(TAG, "Descriptor argument is missing"); + return BLE_ATT_ERR_UNLIKELY; + } - rc = os_mbuf_append(ctxt->om, ctxt->dsc->arg, temp_outlen); - return rc; + int rc; + size_t desc_len = strlen(ctxt->dsc->arg); + if (desc_len > PROTOCOMM_NIMBLE_MAX_PAYLOAD_LEN) { + ESP_LOGE(TAG, "Descriptor value too long: %d", (int)desc_len); + return BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN; + } + + rc = os_mbuf_append(ctxt->om, ctxt->dsc->arg, desc_len); + return rc == 0 ? 0 : BLE_ATT_ERR_INSUFFICIENT_RES; } /* Callback to handle GATT characteristic value Read & Write */ @@ -363,6 +385,20 @@ gatt_svr_chr_access(uint16_t conn_handle, uint16_t attr_handle, return 0; } + if (temp_outlen < 0 || temp_outlen > PROTOCOMM_NIMBLE_MAX_PAYLOAD_LEN) { + ESP_LOGE(TAG, "Invalid response length for attr_handle=%d: %d", attr_handle, (int)temp_outlen); + return BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN; + } + + if (temp_outlen > 0 && temp_outbuf == NULL) { + ESP_LOGE(TAG, "NULL response buffer for attr_handle=%d", attr_handle); + return BLE_ATT_ERR_UNLIKELY; + } + + if (temp_outlen == 0) { + return 0; + } + rc = os_mbuf_append(ctxt->om, temp_outbuf, temp_outlen); return rc == 0 ? 0 : BLE_ATT_ERR_INSUFFICIENT_RES; @@ -388,6 +424,11 @@ gatt_svr_chr_access(uint16_t conn_handle, uint16_t attr_handle, /* Save the length of entire data */ data_len = OS_MBUF_PKTLEN(ctxt->om); + if (data_len == 0 || data_len > PROTOCOMM_NIMBLE_MAX_PAYLOAD_LEN) { + ESP_LOGE(TAG, "Invalid write length: %d", data_len); + free(uuid); + return BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN; + } ESP_LOGD(TAG, "Write attempt for uuid = %s, attr_handle = %d, data_len = %d", ble_uuid_to_str(ctxt->chr->uuid, buf), attr_handle, data_len); @@ -405,9 +446,31 @@ gatt_svr_chr_access(uint16_t conn_handle, uint16_t attr_handle, free(data_buf); return BLE_ATT_ERR_UNLIKELY; } + if (data_buf_len != data_len) { + ESP_LOGE(TAG, "Mbuf flatten length mismatch: expected=%d actual=%d", data_len, data_buf_len); + free(uuid); + free(data_buf); + return BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN; + } - ret = protocomm_req_handle(protoble_internal->pc_ble, - uuid128_to_handler(uuid), + const char *ep_name = uuid128_to_handler(uuid); + if (ep_name == NULL) { + ESP_LOGE(TAG, "No endpoint mapped for characteristic UUID"); + free(uuid); + free(data_buf); + return BLE_ATT_ERR_UNLIKELY; + } + + protocomm_t *pc_ble = protoble_internal->pc_ble; + if (pc_ble == NULL) { + ESP_LOGW(TAG, "Ignoring characteristic access on inactive protocomm transport"); + free(uuid); + free(data_buf); + return BLE_ATT_ERR_UNLIKELY; + } + + ret = protocomm_req_handle(pc_ble, + ep_name, conn_handle, data_buf, data_buf_len, @@ -416,6 +479,15 @@ gatt_svr_chr_access(uint16_t conn_handle, uint16_t attr_handle, free(uuid); free(data_buf); if (ret == ESP_OK) { + if (temp_outlen < 0 || temp_outlen > PROTOCOMM_NIMBLE_MAX_PAYLOAD_LEN) { + ESP_LOGE(TAG, "Invalid protocomm response length: %d", (int)temp_outlen); + free(temp_outbuf); + return BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN; + } + if (temp_outlen > 0 && temp_outbuf == NULL) { + ESP_LOGE(TAG, "Protocomm response buffer is NULL for non-zero length"); + return BLE_ATT_ERR_UNLIKELY; + } /* Save data address and length outbuf and outlen internally */ rc = simple_ble_gatts_set_attr_value(attr_handle, temp_outlen, @@ -426,7 +498,7 @@ gatt_svr_chr_access(uint16_t conn_handle, uint16_t attr_handle, free(temp_outbuf); } - return rc; + return rc == 0 ? 0 : BLE_ATT_ERR_INSUFFICIENT_RES; } else { ESP_LOGE(TAG, "Invalid content received, killing connection"); return BLE_ATT_ERR_INVALID_PDU; @@ -575,16 +647,17 @@ static int simple_ble_start(const simple_ble_cfg_t *cfg) rc = gatt_svr_init(cfg); if (rc != 0) { ESP_LOGE(TAG, "Error initializing GATT server"); - return rc; + goto err_deinit_port; } /* Set device name, configure response data to be sent while advertising */ rc = ble_svc_gap_device_name_set(cfg->device_name); if (rc != 0) { ESP_LOGE(TAG, "Error setting device name"); - return rc; + goto err_deinit_port; } + memset(&resp_data, 0, sizeof(resp_data)); resp_data.name = (void *) ble_svc_gap_device_name(); if (resp_data.name != NULL) { resp_data.name_len = strlen(ble_svc_gap_device_name()); @@ -604,6 +677,12 @@ static int simple_ble_start(const simple_ble_cfg_t *cfg) nimble_port_freertos_init(nimble_host_task); return 0; + +#if MYNEWT_VAL(BLE_GATTS) +err_deinit_port: + nimble_port_deinit(); + return rc; +#endif } /* transport_simple BLE Fn */ @@ -624,28 +703,47 @@ static void transport_simple_ble_disconnect(struct ble_gap_event *event, void *a return; } - if (protoble_internal->pc_ble->sec && - protoble_internal->pc_ble->sec->close_transport_session) { + /* Avoid stale response reuse across sessions. */ + simple_ble_gatts_clear_cached_values(); + + protocomm_t *pc_ble = protoble_internal->pc_ble; + if (pc_ble == NULL) { + ESP_LOGD(TAG, "Protocomm BLE inactive, ignoring disconnect"); + return; + } + + if (pc_ble->sec && pc_ble->sec->close_transport_session) { ret = - protoble_internal->pc_ble->sec->close_transport_session(protoble_internal->pc_ble->sec_inst, event->disconnect.conn.conn_handle); + pc_ble->sec->close_transport_session(pc_ble->sec_inst, event->disconnect.conn.conn_handle); if (ret != ESP_OK) { ESP_LOGE(TAG, "error closing the session after disconnect"); - } else { - protocomm_ble_event_t ble_event = {}; - /* Assign the event type */ - ble_event.evt_type = PROTOCOMM_TRANSPORT_BLE_DISCONNECTED; - /* Set the Disconnection handle */ - ble_event.conn_handle = event->disconnect.conn.conn_handle; - ble_event.disconnect_reason = event->disconnect.reason; - - if (esp_event_post(PROTOCOMM_TRANSPORT_BLE_EVENT, PROTOCOMM_TRANSPORT_BLE_DISCONNECTED, &ble_event, sizeof(protocomm_ble_event_t), portMAX_DELAY) != ESP_OK) { - ESP_LOGE(TAG, "Failed to post transport disconnection event"); - } } } + + protocomm_ble_event_t ble_event = {}; + /* Assign the event type */ + ble_event.evt_type = PROTOCOMM_TRANSPORT_BLE_DISCONNECTED; + /* Set the Disconnection handle */ + ble_event.conn_handle = event->disconnect.conn.conn_handle; + ble_event.disconnect_reason = event->disconnect.reason; + + if (esp_event_post(PROTOCOMM_TRANSPORT_BLE_EVENT, PROTOCOMM_TRANSPORT_BLE_DISCONNECTED, &ble_event, sizeof(protocomm_ble_event_t), portMAX_DELAY) != ESP_OK) { + ESP_LOGE(TAG, "Failed to post transport disconnection event"); + } + protoble_internal->gatt_mtu = BLE_ATT_MTU_DFLT; } +static void simple_ble_gatts_clear_cached_values(void) +{ + struct data_mbuf *cur; + SLIST_FOREACH(cur, &data_mbuf_list, node) { + free(cur->outbuf); + cur->outbuf = NULL; + cur->outlen = 0; + } +} + static void transport_simple_ble_connect(struct ble_gap_event *event, void *arg) { esp_err_t ret; @@ -662,10 +760,15 @@ static void transport_simple_ble_connect(struct ble_gap_event *event, void *arg) return; } - if (protoble_internal->pc_ble->sec && - protoble_internal->pc_ble->sec->new_transport_session) { + protocomm_t *pc_ble = protoble_internal->pc_ble; + if (pc_ble == NULL) { + ESP_LOGD(TAG, "Protocomm BLE inactive, ignoring connect"); + return; + } + + if (pc_ble->sec && pc_ble->sec->new_transport_session) { ret = - protoble_internal->pc_ble->sec->new_transport_session(protoble_internal->pc_ble->sec_inst, event->connect.conn_handle); + pc_ble->sec->new_transport_session(pc_ble->sec_inst, event->connect.conn_handle); if (ret != ESP_OK) { ESP_LOGE(TAG, "error creating the session"); } else { @@ -813,7 +916,7 @@ ble_gatt_add_primary_svcs(struct ble_gatt_svc_def *gatt_db_svcs, int char_count) } static int -populate_gatt_db(struct ble_gatt_svc_def **gatt_db_svcs, const protocomm_ble_config_t *config) +populate_gatt_db(struct ble_gatt_svc_def **gatt_db_svcs, const protocomm_ble_config_t *config, int char_count) { /* Allocate memory for 2 services, 2nd to be all NULL indicating end of * services */ @@ -837,13 +940,13 @@ populate_gatt_db(struct ble_gatt_svc_def **gatt_db_svcs, const protocomm_ble_con memcpy((void *) (*gatt_db_svcs)->uuid, &uuid128, sizeof(ble_uuid128_t)); /* GATT: Add primary service. */ - int rc = ble_gatt_add_primary_svcs(*gatt_db_svcs, config->nu_lookup_count); + int rc = ble_gatt_add_primary_svcs(*gatt_db_svcs, char_count); if (rc != 0) { ESP_LOGE(TAG, "Error adding primary service !!!"); return rc; } - for (int i = 0 ; i < config->nu_lookup_count; i++) { + for (int i = 0 ; i < char_count; i++) { /* GATT: Add characteristics to the service at index no. i*/ rc = ble_gatt_add_characteristics((void *) (*gatt_db_svcs)->characteristics, i); @@ -862,11 +965,30 @@ populate_gatt_db(struct ble_gatt_svc_def **gatt_db_svcs, const protocomm_ble_con return 0; } +static void free_uuid128_name_table(void) +{ + /* Free the uuid_name_table struct list if exists */ + struct uuid128_name_buf *cur; + while (!SLIST_EMPTY(&uuid128_name_list)) { + cur = SLIST_FIRST(&uuid128_name_list); + SLIST_REMOVE_HEAD(&uuid128_name_list, link); + if (cur->uuid128_name_table) { + if (adv_data.uuids128 == (void *)cur->uuid128_name_table) { + adv_data.uuids128 = NULL; + adv_data.num_uuids128 = 0; + } + free(cur->uuid128_name_table); + } + free(cur); + } +} + static void protocomm_ble_cleanup(void) { + free_uuid128_name_table(); if (protoble_internal) { if (protoble_internal->g_nu_lookup) { - for (unsigned i = 0; i < protoble_internal->g_nu_lookup_count; i++) { + for (size_t i = 0; i < protoble_internal->g_nu_lookup_count; i++) { if (protoble_internal->g_nu_lookup[i].name) { free((void *)protoble_internal->g_nu_lookup[i].name); } @@ -918,18 +1040,9 @@ static void free_gatt_ble_misc_memory(simple_ble_cfg_t *ble_config) } free(ble_config); - ble_config = NULL; + ble_cfg_p = NULL; - /* Free the uuid_name_table struct list if exists */ - struct uuid128_name_buf *cur; - while (!SLIST_EMPTY(&uuid128_name_list)) { - cur = SLIST_FIRST(&uuid128_name_list); - SLIST_REMOVE_HEAD(&uuid128_name_list, link); - if (cur->uuid128_name_table) { - free(cur->uuid128_name_table); - } - free(cur); - } + free_uuid128_name_table(); /* Free the data_mbuf list if exists */ struct data_mbuf *curr; @@ -947,11 +1060,34 @@ esp_err_t protocomm_ble_start(protocomm_t *pc, const protocomm_ble_config_t *con return ESP_ERR_INVALID_ARG; } + if (config->manufacturer_data_len > 0 && config->manufacturer_data == NULL) { + ESP_LOGE(TAG, "Manufacturer data length set without data"); + return ESP_ERR_INVALID_ARG; + } + + if (config->nu_lookup_count <= 0 || config->nu_lookup_count > (ssize_t)(INT_MAX - 1)) { + ESP_LOGE(TAG, "Invalid nu_lookup_count: %d", (int)config->nu_lookup_count); + return ESP_ERR_INVALID_ARG; + } + + if (config->manufacturer_data != NULL && + (config->manufacturer_data_len <= 0 || + config->manufacturer_data_len > MAX_BLE_MANUFACTURER_DATA_LEN)) { + ESP_LOGE(TAG, "Invalid manufacturer data length: %d", (int)config->manufacturer_data_len); + return ESP_ERR_INVALID_ARG; + } + if (protoble_internal) { ESP_LOGE(TAG, "Protocomm BLE already started"); return ESP_FAIL; } + size_t endpoint_count = (size_t)config->nu_lookup_count; + if (endpoint_count > (SIZE_MAX / sizeof(protocomm_ble_name_uuid_t))) { + ESP_LOGE(TAG, "Name UUID table size overflow"); + return ESP_ERR_NO_MEM; + } + /* copy the 128 bit service UUID into local buffer to use as base 128 bit * UUID. */ memcpy(ble_uuid_base, config->service_uuid, BLE_UUID128_VAL_LENGTH); @@ -973,17 +1109,14 @@ esp_err_t protocomm_ble_start(protocomm_t *pc, const protocomm_ble_config_t *con if (temp_uuid128_name_buf == NULL) { ESP_LOGE(TAG, "Error allocating memory for UUID128 address database"); + free(svc_uuid128); + adv_data.uuids128 = NULL; + adv_data.num_uuids128 = 0; return ESP_ERR_NO_MEM; } SLIST_INSERT_HEAD(&uuid128_name_list, temp_uuid128_name_buf, link); temp_uuid128_name_buf->uuid128_name_table = svc_uuid128; - if (adv_data.uuids128 == NULL) { - ESP_LOGE(TAG, "Error allocating memory for storing service UUID"); - protocomm_ble_cleanup(); - return ESP_ERR_NO_MEM; - } - /* Store BLE device name internally */ protocomm_ble_device_name = strdup(config->device_name); if (protocomm_ble_device_name == NULL) { @@ -994,8 +1127,14 @@ esp_err_t protocomm_ble_start(protocomm_t *pc, const protocomm_ble_config_t *con /* Store BLE manufacturer data pointer */ if (config->manufacturer_data != NULL) { - protocomm_ble_mfg_data = config->manufacturer_data; - protocomm_ble_mfg_data_len = config->manufacturer_data_len; + protocomm_ble_mfg_data = (uint8_t *)malloc((size_t)config->manufacturer_data_len); + if (protocomm_ble_mfg_data == NULL) { + ESP_LOGE(TAG, "Error allocating memory for manufacturer data"); + protocomm_ble_cleanup(); + return ESP_ERR_NO_MEM; + } + memcpy(protocomm_ble_mfg_data, config->manufacturer_data, (size_t)config->manufacturer_data_len); + protocomm_ble_mfg_data_len = (size_t)config->manufacturer_data_len; } protoble_internal = (_protocomm_ble_internal_t *) calloc(1, sizeof(_protocomm_ble_internal_t)); @@ -1005,16 +1144,22 @@ esp_err_t protocomm_ble_start(protocomm_t *pc, const protocomm_ble_config_t *con return ESP_ERR_NO_MEM; } - protoble_internal->g_nu_lookup_count = config->nu_lookup_count; - protoble_internal->g_nu_lookup = malloc(config->nu_lookup_count * sizeof(protocomm_ble_name_uuid_t)); + protoble_internal->g_nu_lookup_count = endpoint_count; + protoble_internal->g_nu_lookup = calloc(endpoint_count, sizeof(protocomm_ble_name_uuid_t)); if (protoble_internal->g_nu_lookup == NULL) { ESP_LOGE(TAG, "Error allocating internal name UUID table"); protocomm_ble_cleanup(); return ESP_ERR_NO_MEM; } - for (unsigned i = 0; i < protoble_internal->g_nu_lookup_count; i++) { + for (size_t i = 0; i < protoble_internal->g_nu_lookup_count; i++) { protoble_internal->g_nu_lookup[i].uuid = config->nu_lookup[i].uuid; + if (config->nu_lookup[i].name == NULL) { + ESP_LOGE(TAG, "Invalid endpoint name"); + protocomm_ble_cleanup(); + return ESP_ERR_INVALID_ARG; + } + protoble_internal->g_nu_lookup[i].name = strdup(config->nu_lookup[i].name); if (protoble_internal->g_nu_lookup[i].name == NULL) { ESP_LOGE(TAG, "Error allocating internal name UUID entry"); @@ -1051,12 +1196,20 @@ esp_err_t protocomm_ble_start(protocomm_t *pc, const protocomm_ble_config_t *con ble_config->ble_sm_sc = config->ble_sm_sc; if (config->ble_addr != NULL) { - protocomm_ble_addr = config->ble_addr; + protocomm_ble_addr = (uint8_t *)malloc(BLE_ADDR_LEN); + if (protocomm_ble_addr == NULL) { + ESP_LOGE(TAG, "Error allocating memory for BLE address"); + free_gatt_ble_misc_memory(ble_config); + protocomm_ble_cleanup(); + return ESP_ERR_NO_MEM; + } + memcpy(protocomm_ble_addr, config->ble_addr, BLE_ADDR_LEN); } - if (populate_gatt_db(&ble_config->gatt_db, config) != 0) { + if (populate_gatt_db(&ble_config->gatt_db, config, (int)endpoint_count) != 0) { ESP_LOGE(TAG, "Error populating GATT Database"); free_gatt_ble_misc_memory(ble_config); + protocomm_ble_cleanup(); return ESP_ERR_NO_MEM; } @@ -1100,7 +1253,7 @@ esp_err_t protocomm_ble_stop(protocomm_t *pc) /* Keep BT stack on, but terminate the connection after provisioning */ rc = ble_gap_terminate(s_cached_conn_handle, BLE_ERR_REM_USER_CONN_TERM); if (rc) { - ESP_LOGI(TAG, "Error in terminating connection rc = %d",rc); + ESP_LOGI(TAG, "Error in terminating connection rc = %d", rc); } free_gatt_ble_misc_memory(ble_cfg_p); ble_callbacks_active = false; @@ -1113,6 +1266,9 @@ esp_err_t protocomm_ble_stop(protocomm_t *pc) ret = nimble_port_stop(); if (ret == 0) { nimble_port_deinit(); + } else { + protoble_internal->pc_ble = pc; + return ret; } free_gatt_ble_misc_memory(ble_cfg_p); ble_callbacks_active = false;