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

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

See merge request espressif/esp-idf!44299
This commit is contained in:
Rahul Tank
2025-12-23 15:52:59 +05:30
5 changed files with 192 additions and 42 deletions

View File

@@ -143,6 +143,11 @@ typedef struct protocomm_ble_config {
*/
uint8_t *ble_addr;
/**
* Flag to keep BLE on
*/
unsigned keep_ble_on:1;
/**
* BLE characteristic notify flag
*/

View File

@@ -14,6 +14,7 @@
#include <esp_gap_ble_api.h>
#include <esp_gatts_api.h>
#include <esp_bt_main.h>
#include "esp_bt_device.h"
#include <esp_gatt_common_api.h>
#include "simple_ble.h"
@@ -30,6 +31,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;
@@ -250,7 +256,8 @@ esp_err_t simple_ble_start(simple_ble_cfg_t *cfg)
}
#endif
ret = esp_bluedroid_init();
esp_bluedroid_config_t bluedroid_cfg = BT_BLUEDROID_INIT_CONFIG_DEFAULT();
ret = esp_bluedroid_init_with_cfg(&bluedroid_cfg);
if (ret) {
ESP_LOGE(TAG, "%s init bluetooth failed %d", __func__, ret);
return ret;

View File

@@ -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

View File

@@ -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>
@@ -71,6 +72,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,
@@ -121,6 +137,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;
@@ -140,6 +159,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) {
@@ -206,7 +237,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;
}
@@ -254,6 +286,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) {
@@ -298,6 +341,15 @@ 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) {
@@ -336,12 +388,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,
@@ -368,12 +428,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,
@@ -396,6 +464,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;
}
@@ -485,6 +556,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++) {
@@ -608,6 +680,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;
}
@@ -634,31 +709,44 @@ 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");
}
#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;
}

View File

@@ -5,6 +5,7 @@
*/
#include <sys/param.h>
#include <stdbool.h>
#include <esp_log.h>
#include <string.h>
#include <inttypes.h>
@@ -73,6 +74,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;
@@ -283,6 +289,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++) {
@@ -301,6 +310,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;
@@ -329,6 +343,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",
@@ -593,6 +612,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 =
@@ -626,6 +650,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 =
@@ -649,6 +678,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;
}
@@ -1038,37 +1070,43 @@ 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);
}
protoble_internal->pc_ble = NULL;
#ifdef CONFIG_ESP_PROTOCOMM_KEEP_BLE_ON_AFTER_BLE_STOP
/* Keep BLE host running after provisioning; leave resources intact */
#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);
if (rc) {
ESP_LOGI(TAG, "Error in terminating connection rc = %d",rc);
}
free_gatt_ble_misc_memory(ble_cfg_p);
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);
}
#endif // CONFIG_ESP_PROTOCOMM_DISCONNECT_AFTER_BLE_STOP
ESP_LOGD(TAG, "Keeping BLE stack running after protocomm stop");
#else
/* If flag is enabled, don't stop the stack. User application can start a new advertising to perform its BT activities */
ret = nimble_port_stop();
if (ret == 0) {
nimble_port_deinit();
}
free_gatt_ble_misc_memory(ble_cfg_p);
/* 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);
ble_callbacks_active = false;
#endif // CONFIG_ESP_PROTOCOMM_KEEP_BLE_ON_AFTER_BLE_STOP
if (!ble_callbacks_active) {
protocomm_ble_cleanup();
return ret;
}
return ESP_ERR_INVALID_ARG;
return ret;
}