Mirrors/esp-idf
1
0
Fork 0
mirror of https://github.com/espressif/esp-idf.git synced 2026-09-22 13:01:16 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix(esp_event): skip dispatch for internal cleanup events (SEC-221)

Browse Source 
After processing an esp_event_handler_cleanup sentinel, execution fell
through into the regular dispatch block. Every loop-level (ANY_BASE/
ANY_ID) handler was invoked with base="cleanup" and event_data pointing
at the internal esp_event_remove_handler_context_t struct.

Consequences:
- Information disclosure: internal handler addresses and loop instance
  pointer are exposed to every loop-level handler.
- UAF: if a handler stores event_data for later use, post_instance_delete
  frees the ctx, turning the stored pointer into a dangling reference.
- Logic corruption: handlers that switch on base with a default branch
  misbehave on every unregister anywhere in the system.

Fix: wrap the regular dispatch block in an else clause so it is skipped
entirely for cleanup events. post_instance_delete, ticks accounting, and
xSemaphoreGiveRecursive remain in the shared tail executed for both paths.

Closes SEC_221
This commit is contained in:
Konstantin Kondrashov
2026-07-06 16:46:16 +03:00
parent 086faab0c5
commit c0fc78ca26
1 changed files with 37 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

74
components/esp_event/esp_event.c
View File

@@ -645,6 +645,8 @@ esp_err_t esp_event_loop_run(esp_event_loop_handle_t event_loop, TickType_t tick
// The event has already been unqueued, so ensure it gets executed.
xSemaphoreTakeRecursive(loop->mutex, portMAX_DELAY);
bool exec = false;
// check if the event retrieve from the queue is the internal event that is
// triggered when a handler needs to be removed..
if (post.base == esp_event_handler_cleanup) {
@@ -658,47 +660,45 @@ esp_err_t esp_event_loop_run(esp_event_loop_handle_t event_loop, TickType_t tick
if (ctx->legacy) {
free(ctx->handler_ctx);
}
}
} else {
loop->running_task = xTaskGetCurrentTaskHandle();
loop->running_task = xTaskGetCurrentTaskHandle();
esp_event_handler_node_t *handler, *temp_handler;
esp_event_loop_node_t *loop_node, *temp_node;
esp_event_base_node_t *base_node, *temp_base;
esp_event_id_node_t *id_node, *temp_id_node;
bool exec = false;
esp_event_handler_node_t *handler, *temp_handler;
esp_event_loop_node_t *loop_node, *temp_node;
esp_event_base_node_t *base_node, *temp_base;
esp_event_id_node_t *id_node, *temp_id_node;
SLIST_FOREACH_SAFE(loop_node, &(loop->loop_nodes), next, temp_node) {
// Execute loop level handlers
SLIST_FOREACH_SAFE(handler, &(loop_node->handlers), next, temp_handler) {
if (!handler->unregistered) {
handler_execute(loop, handler, post);
exec |= true;
}
}
SLIST_FOREACH_SAFE(base_node, &(loop_node->base_nodes), next, temp_base) {
if (base_node->base == post.base) {
// Execute base level handlers
SLIST_FOREACH_SAFE(handler, &(base_node->handlers), next, temp_handler) {
if (!handler->unregistered) {
handler_execute(loop, handler, post);
exec |= true;
}
SLIST_FOREACH_SAFE(loop_node, &(loop->loop_nodes), next, temp_node) {
// Execute loop level handlers
SLIST_FOREACH_SAFE(handler, &(loop_node->handlers), next, temp_handler) {
if (!handler->unregistered) {
handler_execute(loop, handler, post);
exec |= true;
}
}
SLIST_FOREACH_SAFE(id_node, &(base_node->id_nodes), next, temp_id_node) {
if (id_node->id == post.id) {
// Execute id level handlers
SLIST_FOREACH_SAFE(handler, &(id_node->handlers), next, temp_handler) {
if (!handler->unregistered) {
handler_execute(loop, handler, post);
exec |= true;
}
SLIST_FOREACH_SAFE(base_node, &(loop_node->base_nodes), next, temp_base) {
if (base_node->base == post.base) {
// Execute base level handlers
SLIST_FOREACH_SAFE(handler, &(base_node->handlers), next, temp_handler) {
if (!handler->unregistered) {
handler_execute(loop, handler, post);
exec |= true;
}
}
SLIST_FOREACH_SAFE(id_node, &(base_node->id_nodes), next, temp_id_node) {
if (id_node->id == post.id) {
// Execute id level handlers
SLIST_FOREACH_SAFE(handler, &(id_node->handlers), next, temp_handler) {
if (!handler->unregistered) {
handler_execute(loop, handler, post);
exec |= true;
}
}
// Skip to next base node
break;
}
// Skip to next base node
break;
}
}
}
@@ -726,7 +726,7 @@ esp_err_t esp_event_loop_run(esp_event_loop_handle_t event_loop, TickType_t tick
xSemaphoreGiveRecursive(loop->mutex);
if (!exec) {
if (!exec && base != esp_event_handler_cleanup) {
// No handlers were registered, not even loop/base level handlers
ESP_LOGD(TAG, "no handlers have been registered for event %s:%"PRIu32" posted to loop %p", base, id, event_loop);
}