diff --git a/components/protocomm/src/crypto/srp6a/esp_srp.c b/components/protocomm/src/crypto/srp6a/esp_srp.c index 9109cbe6c0e..28ebf38636c 100644 --- a/components/protocomm/src/crypto/srp6a/esp_srp.c +++ b/components/protocomm/src/crypto/srp6a/esp_srp.c @@ -549,6 +549,10 @@ 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)"); + goto error; + } hexdump_mpi("u", u); /* S = (A v^u)^b */ diff --git a/components/protocomm/src/transports/protocomm_nimble.c b/components/protocomm/src/transports/protocomm_nimble.c index c1c674de158..70199deda62 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 @@ -34,6 +35,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 * ********************************************************/ @@ -67,7 +75,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; @@ -116,6 +124,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 */ @@ -192,6 +201,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; @@ -258,9 +272,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; @@ -296,12 +307,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; @@ -322,11 +335,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 */ @@ -362,6 +384,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; @@ -387,6 +423,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); @@ -404,9 +445,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, @@ -415,6 +478,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, @@ -425,7 +497,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; @@ -570,16 +642,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()); @@ -599,6 +672,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 */ @@ -619,28 +698,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; @@ -657,10 +755,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 { @@ -808,7 +911,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 */ @@ -832,13 +935,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); @@ -857,11 +960,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); } @@ -913,18 +1035,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; @@ -942,11 +1055,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); @@ -968,17 +1104,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) { @@ -989,8 +1122,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)); @@ -1000,16 +1139,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"); @@ -1046,12 +1191,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; } @@ -1108,6 +1261,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;