Merge branch 'bugfix/bug_bounty_ble_issue_v5.1' into 'release/v5.1'

fix(protocomm): Add security checks for buffer overflow and incorrect length handling (v5.1)

See merge request espressif/esp-idf!44383
This commit is contained in:
Mahavir Jain
2025-12-23 15:22:10 +05:30
5 changed files with 205 additions and 38 deletions
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2018-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2018-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -142,6 +142,11 @@ typedef struct protocomm_ble_config {
* BLE address
*/
uint8_t *ble_addr;
/**
* Flag to keep BLE on
*/
unsigned keep_ble_on:1;
} protocomm_ble_config_t;
/**
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -30,6 +30,11 @@ static esp_bd_addr_t s_cached_remote_bda = {0x0,};
#define adv_config_flag (1 << 0)
#define scan_rsp_config_flag (1 << 1)
uint8_t get_keep_ble_on()
{
return g_ble_cfg_p->keep_ble_on;
}
const uint8_t *simple_ble_get_uuid128(uint16_t handle)
{
const uint8_t *uuid128_ptr;
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -53,10 +53,22 @@ typedef struct {
unsigned ble_sm_sc:1;
/** BLE Address */
uint8_t *ble_addr;
/** Flag to keep BLE on */
unsigned keep_ble_on:1;
} simple_ble_cfg_t;
/**
* @brief Get the current BLE keep-on status
*
* This function returns the current value of the `keep_ble_on` flag
* from the global BLE configuration structure.
*
* @return uint8_t Current status of the `keep_ble_on` flag
*/
uint8_t get_keep_ble_on(void);
/** Initialize a simple ble connection
*
* This function allocates memory and returns a pointer to the
@@ -1,10 +1,11 @@
/*
* SPDX-FileCopyrightText: 2018-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2018-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#include <sys/param.h>
#include <stdbool.h>
#include <esp_log.h>
#include <esp_gatt_common_api.h>
#include <esp_gap_bt_api.h>
@@ -68,6 +69,21 @@ typedef struct _protocomm_ble {
static _protocomm_ble_internal_t *protoble_internal;
static bool protocomm_ble_transport_active(void)
{
return (protoble_internal != NULL) && (protoble_internal->pc_ble != NULL);
}
static void protocomm_ble_reset_prepare_write(void)
{
if (prepare_write_env.prepare_buf) {
free(prepare_write_env.prepare_buf);
prepare_write_env.prepare_buf = NULL;
}
prepare_write_env.prepare_len = 0;
prepare_write_env.handle = 0;
}
// config adv data
static esp_ble_adv_data_t adv_config = {
.set_scan_rsp = false,
@@ -118,6 +134,9 @@ static const uint16_t *uuid128_to_16(const uint8_t *uuid128)
static const char *handle_to_handler(uint16_t handle)
{
if (protoble_internal == NULL) {
return NULL;
}
const uint8_t *uuid128 = simple_ble_get_uuid128(handle);
if (!uuid128) {
return NULL;
@@ -137,6 +156,18 @@ static void transport_simple_ble_read(esp_gatts_cb_event_t event, esp_gatt_if_t
static uint16_t max_read_len = 0;
esp_gatt_status_t status = ESP_OK;
if (!protocomm_ble_transport_active()) {
read_len = 0;
max_read_len = 0;
read_buf = NULL;
esp_err_t err = esp_ble_gatts_send_response(gatts_if, param->read.conn_id,
param->read.trans_id, ESP_GATT_ERROR, NULL);
if (err != ESP_OK) {
ESP_LOGE(TAG, "Send response error in read");
}
return;
}
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) {
@@ -203,7 +234,8 @@ static esp_err_t prepare_write_event_env(esp_gatt_if_t gatts_if,
memcpy(prepare_write_env.prepare_buf + param->write.offset,
param->write.value,
param->write.len);
prepare_write_env.prepare_len += 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;
}
@@ -251,6 +283,17 @@ static void transport_simple_ble_write(esp_gatts_cb_event_t event, esp_gatt_if_t
ESP_LOGD(TAG, "Inside write with session - %d on attr handle = %d \nlen = %d, is_prep = %d",
param->write.conn_id, param->write.handle, param->write.len, param->write.is_prep);
if (!protocomm_ble_transport_active()) {
ESP_LOGW(TAG, "Ignoring write on inactive protocomm transport");
protocomm_ble_reset_prepare_write();
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);
return;
}
if (param->write.is_prep) {
ret = prepare_write_event_env(gatts_if, param);
if (ret != ESP_OK) {
@@ -295,9 +338,28 @@ static void transport_simple_ble_exec_write(esp_gatts_cb_event_t event, esp_gatt
ssize_t outlen = 0;
ESP_LOGD(TAG, "Inside exec_write w/ session - %d", param->exec_write.conn_id);
if (!protocomm_ble_transport_active()) {
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) {
/* Validate prepare_len to prevent buffer over-read (defense-in-depth) */
if (prepare_write_env.prepare_len > PREPARE_BUF_MAX_SIZE) {
ESP_LOGE(TAG, "Invalid prepare_len %d, max allowed %d",
prepare_write_env.prepare_len, PREPARE_BUF_MAX_SIZE);
protocomm_ble_reset_prepare_write();
esp_ble_gatts_close(gatts_if, param->exec_write.conn_id);
esp_ble_gatts_send_response(gatts_if, param->exec_write.conn_id,
param->exec_write.trans_id, ESP_GATT_ERROR, NULL);
return;
}
err = protocomm_req_handle(protoble_internal->pc_ble,
handle_to_handler(prepare_write_env.handle),
param->exec_write.conn_id,
@@ -333,12 +395,20 @@ static void transport_simple_ble_disconnect(esp_gatts_cb_event_t event, esp_gatt
esp_err_t ret;
ESP_LOGD(TAG, "Inside disconnect w/ session - %d", param->disconnect.conn_id);
/* Drop any staged prepare-write data when a connection ends */
protocomm_ble_reset_prepare_write();
/* Ignore BLE events received after protocomm layer is stopped */
if (protoble_internal == NULL) {
ESP_LOGI(TAG,"Protocomm layer has already stopped");
return;
}
if (!protocomm_ble_transport_active()) {
ESP_LOGD(TAG, "Protocomm BLE inactive, ignoring disconnect");
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,
@@ -365,12 +435,20 @@ static void transport_simple_ble_connect(esp_gatts_cb_event_t event, esp_gatt_if
esp_err_t ret;
ESP_LOGD(TAG, "Inside BLE connect w/ conn_id - %d", param->connect.conn_id);
/* Start each connection with a clean prepare-write buffer */
protocomm_ble_reset_prepare_write();
/* Ignore BLE events received after protocomm layer is stopped */
if (protoble_internal == NULL) {
ESP_LOGI(TAG,"Protocomm layer has already stopped");
return;
}
if (!protocomm_ble_transport_active()) {
ESP_LOGD(TAG, "Protocomm BLE inactive, ignoring connect");
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,
@@ -393,6 +471,9 @@ static void transport_simple_ble_connect(esp_gatts_cb_event_t event, esp_gatt_if
static void transport_simple_ble_set_mtu(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, esp_ble_gatts_cb_param_t *param)
{
if (!protocomm_ble_transport_active()) {
return;
}
protoble_internal->gatt_mtu = param->mtu.mtu;
return;
}
@@ -477,6 +558,7 @@ static ssize_t populate_gatt_db(esp_gatts_attr_db_t **gatt_db_generated)
static void protocomm_ble_cleanup(void)
{
protocomm_ble_reset_prepare_write();
if (protoble_internal) {
if (protoble_internal->g_nu_lookup) {
for (unsigned i = 0; i < protoble_internal->g_nu_lookup_count; i++) {
@@ -599,6 +681,9 @@ esp_err_t protocomm_ble_start(protocomm_t *pc, const protocomm_ble_config_t *con
ble_config->ble_bonding = config->ble_bonding;
ble_config->ble_sm_sc = config->ble_sm_sc;
/* Set parameter to keep BLE on */
ble_config->keep_ble_on = config->keep_ble_on;
if (config->ble_addr != NULL) {
ble_config->ble_addr = protocomm_ble_addr;
}
@@ -625,31 +710,45 @@ esp_err_t protocomm_ble_start(protocomm_t *pc, const protocomm_ble_config_t *con
esp_err_t protocomm_ble_stop(protocomm_t *pc)
{
if ((pc != NULL) &&
(protoble_internal != NULL ) &&
(pc == protoble_internal->pc_ble)) {
esp_err_t ret = ESP_OK;
if ((pc == NULL) ||
(protoble_internal == NULL ) ||
(pc != protoble_internal->pc_ble)) {
return ESP_ERR_INVALID_ARG;
}
#ifdef CONFIG_ESP_PROTOCOMM_KEEP_BLE_ON_AFTER_BLE_STOP
esp_err_t ret = ESP_OK;
bool ble_callbacks_active = true;
uint8_t protocomm_keep_ble_on = get_keep_ble_on();
protoble_internal->pc_ble = NULL;
if (protocomm_keep_ble_on) {
#ifdef CONFIG_ESP_PROTOCOMM_DISCONNECT_AFTER_BLE_STOP
/* Keep BT stack on, but terminate the connection after provisioning */
ret = simple_ble_disconnect();
if (ret) {
ESP_LOGE(TAG, "BLE disconnect failed");
}
simple_ble_deinit();
#endif // CONFIG_ESP_PROTOCOMM_DISCONNECT_AFTER_BLE_STOP
ret = simple_ble_disconnect();
if (ret) {
ESP_LOGE(TAG, "BLE disconnect failed");
}
simple_ble_deinit();
ble_callbacks_active = false;
#else
ESP_LOGD(TAG, "Keeping BLE stack running after protocomm stop");
#endif // CONFIG_ESP_PROTOCOMM_DISCONNECT_AFTER_BLE_STOP
}
else {
/* If flag is not enabled, stop the stack. */
ret = simple_ble_stop();
if (ret) {
ESP_LOGE(TAG, "BLE stop failed");
}
simple_ble_deinit();
#endif // CONFIG_ESP_PROTOCOMM_KEEP_BLE_ON_AFTER_BLE_STOP
protocomm_ble_cleanup();
return ret;
ble_callbacks_active = false;
}
return ESP_ERR_INVALID_ARG;
protocomm_ble_reset_prepare_write();
if (!ble_callbacks_active) {
protocomm_ble_cleanup();
}
return ret;
}
@@ -5,6 +5,7 @@
*/
#include <sys/param.h>
#include <stdbool.h>
#include <esp_log.h>
#include <string.h>
#include <inttypes.h>
@@ -72,6 +73,11 @@ typedef struct _protocomm_ble {
} _protocomm_ble_internal_t;
static _protocomm_ble_internal_t *protoble_internal;
static bool protocomm_ble_transport_active(void)
{
return (protoble_internal != NULL) && (protoble_internal->pc_ble != NULL);
}
static struct ble_gap_adv_params adv_params;
static char *protocomm_ble_device_name;
static struct ble_hs_adv_fields adv_data, resp_data;
@@ -133,6 +139,8 @@ typedef struct {
unsigned ble_link_encryption:1;
/** BLE address */
uint8_t *ble_addr;
/** Flag to keep BLE on */
unsigned keep_ble_on:1;
} simple_ble_cfg_t;
static simple_ble_cfg_t *ble_cfg_p;
@@ -272,6 +280,9 @@ simple_ble_gap_event(struct ble_gap_event *event, void *arg)
/* Gets `g_nu_lookup name handler` from 128 bit UUID */
static const char *uuid128_to_handler(uint8_t *uuid)
{
if (!protocomm_ble_transport_active()) {
return NULL;
}
/* 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++) {
@@ -290,6 +301,11 @@ static int
gatt_svr_dsc_access(uint16_t conn_handle, uint16_t attr_handle, struct
ble_gatt_access_ctxt *ctxt, void *arg)
{
if (!protocomm_ble_transport_active()) {
ESP_LOGW(TAG, "Ignoring descriptor access on inactive protocomm transport");
return BLE_ATT_ERR_UNLIKELY;
}
if (ctxt->op != BLE_GATT_ACCESS_OP_READ_DSC) {
ESP_LOGE(TAG, "Invalid operation on Read-only Descriptor");
return BLE_ATT_ERR_UNLIKELY;
@@ -318,6 +334,11 @@ gatt_svr_chr_access(uint16_t conn_handle, uint16_t attr_handle,
uint16_t data_len = 0;
uint16_t data_buf_len = 0;
if (!protocomm_ble_transport_active()) {
ESP_LOGW(TAG, "Ignoring characteristic access on inactive protocomm transport");
return BLE_ATT_ERR_UNLIKELY;
}
switch (ctxt->op) {
case BLE_GATT_ACCESS_OP_READ_CHR:
ESP_LOGD(TAG, "Read attempted for characteristic UUID = %s, attr_handle = %d",
@@ -582,6 +603,11 @@ static void transport_simple_ble_disconnect(struct ble_gap_event *event, void *a
return;
}
if (!protocomm_ble_transport_active()) {
ESP_LOGD(TAG, "Protocomm BLE inactive, ignoring disconnect");
return;
}
if (protoble_internal->pc_ble->sec &&
protoble_internal->pc_ble->sec->close_transport_session) {
ret =
@@ -615,6 +641,11 @@ static void transport_simple_ble_connect(struct ble_gap_event *event, void *arg)
return;
}
if (!protocomm_ble_transport_active()) {
ESP_LOGD(TAG, "Protocomm BLE inactive, ignoring connect");
return;
}
if (protoble_internal->pc_ble->sec &&
protoble_internal->pc_ble->sec->new_transport_session) {
ret =
@@ -638,6 +669,9 @@ static void transport_simple_ble_connect(struct ble_gap_event *event, void *arg)
static void transport_simple_ble_set_mtu(struct ble_gap_event *event, void *arg)
{
if (!protocomm_ble_transport_active()) {
return;
}
protoble_internal->gatt_mtu = event->mtu.value;
return;
}
@@ -1005,6 +1039,8 @@ esp_err_t protocomm_ble_start(protocomm_t *pc, const protocomm_ble_config_t *con
return ESP_ERR_NO_MEM;
}
ble_config->keep_ble_on = config->keep_ble_on;
esp_err_t err = simple_ble_start(ble_config);
ESP_LOGD(TAG, "Free Heap size after simple_ble_start= %" PRIu32, esp_get_free_heap_size());
@@ -1022,18 +1058,23 @@ esp_err_t protocomm_ble_start(protocomm_t *pc, const protocomm_ble_config_t *con
esp_err_t protocomm_ble_stop(protocomm_t *pc)
{
ESP_LOGD(TAG, "protocomm_ble_stop called here...");
if ((pc != NULL) &&
(protoble_internal != NULL ) &&
(pc == protoble_internal->pc_ble)) {
esp_err_t ret = ESP_OK;
if ((pc == NULL) ||
(protoble_internal == NULL ) ||
(pc != protoble_internal->pc_ble)) {
return ESP_ERR_INVALID_ARG;
}
esp_err_t rc = ble_gap_adv_stop();
if (rc) {
ESP_LOGD(TAG, "Error in stopping advertisement with err code = %d",
rc);
}
esp_err_t ret = ESP_OK;
bool ble_callbacks_active = true;
esp_err_t rc = ble_gap_adv_stop();
if (rc) {
ESP_LOGD(TAG, "Error in stopping advertisement with err code = %d",
rc);
}
#ifdef CONFIG_ESP_PROTOCOMM_KEEP_BLE_ON_AFTER_BLE_STOP
protoble_internal->pc_ble = NULL;
if (ble_cfg_p->keep_ble_on) {
#ifdef CONFIG_ESP_PROTOCOMM_DISCONNECT_AFTER_BLE_STOP
/* Keep BT stack on, but terminate the connection after provisioning */
rc = ble_gap_terminate(s_cached_conn_handle, BLE_ERR_REM_USER_CONN_TERM);
@@ -1041,18 +1082,23 @@ esp_err_t protocomm_ble_stop(protocomm_t *pc)
ESP_LOGI(TAG, "Error in terminating connection rc = %d",rc);
}
free_gatt_ble_misc_memory(ble_cfg_p);
#endif // CONFIG_ESP_PROTOCOMM_DISCONNECT_AFTER_BLE_STOP
ble_callbacks_active = false;
#else
/* If flag is enabled, don't stop the stack. User application can start a new advertising to perform its BT activities */
ESP_LOGD(TAG, "Keeping BLE stack running after protocomm stop");
#endif // CONFIG_ESP_PROTOCOMM_DISCONNECT_AFTER_BLE_STOP
}
else {
/* Stop BLE stack and tear down GATT metadata */
ret = nimble_port_stop();
if (ret == 0) {
nimble_port_deinit();
}
free_gatt_ble_misc_memory(ble_cfg_p);
#endif // CONFIG_ESP_PROTOCOMM_KEEP_BLE_ON_AFTER_BLE_STOP
protocomm_ble_cleanup();
return ret;
ble_callbacks_active = false;
}
return ESP_ERR_INVALID_ARG;
if (!ble_callbacks_active) {
protocomm_ble_cleanup();
}
return ret;
}