fix(esp_tee): Add additional input validation checks for TEE service calls

This commit is contained in:
Laukik Hase
2026-06-25 19:27:16 +05:30
parent 00cff977c6
commit 9ce63d8dfa
5 changed files with 50 additions and 12 deletions

View File

@@ -256,10 +256,20 @@ esp_err_t __wrap_esp_ds_start_sign(const void *message,
if (esp_ds_ctx != NULL) {
*esp_ds_ctx = malloc(sizeof(esp_ds_context_t));
if (!*esp_ds_ctx) {
esp_crypto_ds_lock_release();
return ESP_ERR_NO_MEM;
}
}
return esp_tee_service_call(5, SS_ESP_DS_START_SIGN, message, data, key_id, esp_ds_ctx);
esp_err_t err = esp_tee_service_call(5, SS_ESP_DS_START_SIGN, message, data, key_id, esp_ds_ctx);
if (err != ESP_OK) {
if (esp_ds_ctx != NULL) {
free(*esp_ds_ctx);
*esp_ds_ctx = NULL;
}
esp_crypto_ds_lock_release();
}
return err;
}
bool __wrap_esp_ds_is_busy(void)

View File

@@ -583,6 +583,13 @@ int _ss_esp_tee_ota_end(void)
/* ---------------------------------------------- Secure Storage ------------------------------------------------- */
/* NOTE: The key-name pointers here (cfg->id/ctx->key_id) are REE-supplied, NULL-terminated
* NVS key names used read-only for key lookup (NVS compares them with strncmp bounded to
* NVS_KEY_NAME_MAX_SIZE-1) — never written through, never used as a register base.
* Pointing one at TEE memory yields at most a load-fault DoS or a useless presence oracle,
* so they are left unchecked. Argument checks cost code size and add latency to every
* service call, so we keep only the ones that close a real REE->TEE read/write/control-flow gap.
*/
esp_err_t _ss_esp_tee_sec_storage_clear_key(const char *key_id)
{
return esp_tee_sec_storage_clear_key(key_id);

View File

@@ -145,6 +145,15 @@ void _ss_esprv_int_set_vectored(int rv_int_num, bool vectored)
/* ---------------------------------------------- RTC_WDT ------------------------------------------------- */
static bool is_wdt_dev_valid(const void *dev)
{
return (dev == (const void *)&TIMERG0)
#if TIMG_LL_GET(INST_NUM) >= 2
|| (dev == (const void *)&TIMERG1)
#endif
|| (dev == (const void *)RWDT_DEV_GET());
}
void _ss_wdt_hal_init(wdt_hal_context_t *hal, wdt_inst_t wdt_inst, uint32_t prescaler, bool enable_intr)
{
bool valid_addr = esp_tee_buf_in_ree(hal, sizeof(wdt_hal_context_t));
@@ -159,7 +168,8 @@ void _ss_wdt_hal_init(wdt_hal_context_t *hal, wdt_inst_t wdt_inst, uint32_t pres
void _ss_wdt_hal_deinit(wdt_hal_context_t *hal)
{
bool valid_addr = esp_tee_buf_in_ree(hal, sizeof(wdt_hal_context_t));
bool valid_addr = (esp_tee_buf_in_ree(hal, sizeof(wdt_hal_context_t)) &&
is_wdt_dev_valid(hal->mwdt_dev));
if (!valid_addr) {
return;
@@ -171,6 +181,14 @@ void _ss_wdt_hal_deinit(wdt_hal_context_t *hal)
/* ---------------------------------------------- Secure Storage ------------------------------------------------- */
/* NOTE: The key-name pointers here (cfg->id/ctx->key_id) are REE-supplied, NULL-terminated
* NVS key names used read-only for key lookup (NVS compares them with strncmp bounded to
* NVS_KEY_NAME_MAX_SIZE-1) — never written through, never used as a register base.
* Pointing one at TEE memory yields at most a load-fault DoS or a useless presence oracle,
* so they are left unchecked. Argument checks cost code size and add latency to every
* service call, so we keep only the ones that close a real REE->TEE read/write/control-flow gap.
* The buffers alongside these ARE validated, since the TEE reads/writes them.
*/
esp_err_t _ss_esp_tee_sec_storage_ecdsa_sign(const esp_tee_sec_storage_key_cfg_t *cfg, const uint8_t *hash, size_t hlen, esp_tee_sec_storage_ecdsa_sign_t *out_sign)
{
bool valid_addr = (esp_tee_buf_in_ree(cfg, sizeof(esp_tee_sec_storage_key_cfg_t)) &&
@@ -337,7 +355,10 @@ static bool is_flash_addr_readable(uint32_t paddr, uint32_t len)
static bool is_spi_host_in_ree(spi_flash_host_inst_t *host)
{
return esp_tee_buf_in_ree(host, sizeof(spi_flash_hal_context_t));
const spi_flash_hal_context_t *ctx = (const spi_flash_hal_context_t *)host;
return (esp_tee_buf_in_ree(host, sizeof(spi_flash_hal_context_t)) &&
ctx->spi == spi_flash_ll_get_hw(SPI1_HOST));
}
static bool is_spi_trans_valid(spi_flash_host_inst_t *host, spi_flash_trans_t *trans)

View File

@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2024-2026 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -12,6 +12,7 @@
#include "soc/periph_defs.h"
#include "soc/interrupts.h"
#include "soc/interrupt_reg.h"
#include "soc/soc_caps.h"
#include "esp_tee.h"
#include "esp_tee_intr.h"
@@ -34,6 +35,10 @@ static uint32_t protected_sources[INTR_SET_COUNT];
bool esp_tee_is_intr_src_protected(int source)
{
if (source < 0 || source >= ETS_MAX_INTR_SOURCE) {
return false;
}
uint32_t base = source / INTR_SET_SIZE;
uint32_t offset = source % INTR_SET_SIZE;
@@ -57,14 +62,9 @@ void tee_unhandled_interrupt(void *arg)
/* Interrupt Matrix configuration API to call from non-secure world */
void esp_tee_route_intr_matrix(int cpu_no, uint32_t model_num, uint32_t intr_num)
{
if (esp_tee_is_intr_src_protected(model_num) || intr_num == TEE_SECURE_INUM) {
if (model_num >= ETS_MAX_INTR_SOURCE || esp_tee_is_intr_src_protected(model_num)) {
return;
}
#if SOC_INT_CLIC_SUPPORTED
if (intr_num == TEE_PASS_INUM) {
return;
}
#endif
esp_rom_route_intr_matrix(cpu_no, model_num, intr_num);
ESP_LOGV(TAG, "Connected src %d to int %d (cpu %d)", model_num, intr_num, cpu_no);

View File

@@ -2,8 +2,8 @@
# builds across various configurations - and is not intended for production use.
# Reducing TEE IRAM size
# 29.5KB
CONFIG_SECURE_TEE_IRAM_SIZE=0x7600
# 30KB
CONFIG_SECURE_TEE_IRAM_SIZE=0x7800
# TEE Secure Storage: Release mode
CONFIG_SECURE_TEE_SEC_STG_MODE_RELEASE=y