From 2e6f9b8b42751e51fcc4dd560078f97009e1fd66 Mon Sep 17 00:00:00 2001 From: Ashish Sharma Date: Fri, 26 Jun 2026 19:12:40 +0800 Subject: [PATCH] fix(mbedtls): validate crypto input lengths (TEE OOB, auth-bypass, overflows) --- .../mbedtls/esp_crt_bundle/esp_crt_bundle.c | 22 ++++++++++++++-- components/mbedtls/port/aes/esp_aes.c | 19 +++++++++++++- components/mbedtls/port/aes/esp_aes_gcm.c | 23 ++++++++++++++++- components/mbedtls/port/ecc/esp_ecc.c | 25 ++++++++++++++++++- 4 files changed, 84 insertions(+), 5 deletions(-) diff --git a/components/mbedtls/esp_crt_bundle/esp_crt_bundle.c b/components/mbedtls/esp_crt_bundle/esp_crt_bundle.c index 0b271b73dc0..49db59d6e9e 100644 --- a/components/mbedtls/esp_crt_bundle/esp_crt_bundle.c +++ b/components/mbedtls/esp_crt_bundle/esp_crt_bundle.c @@ -101,9 +101,11 @@ static const uint8_t* esp_crt_get_key(const cert_t cert) return esp_crt_get_name(cert) + esp_crt_get_name_len(cert); } -static uint16_t esp_crt_get_len(const cert_t cert) +static uint32_t esp_crt_get_len(const cert_t cert) { - return CRT_HEADER_SIZE + esp_crt_get_name_len(cert) + esp_crt_get_key_len(cert); + /* Widened to uint32_t: name_len and key_len are each uint16_t, so their sum plus the + * header can exceed UINT16_MAX and would otherwise wrap, under-reporting the cert size. */ + return (uint32_t)CRT_HEADER_SIZE + (uint32_t)esp_crt_get_name_len(cert) + (uint32_t)esp_crt_get_key_len(cert); } static uint32_t esp_crt_get_cert_offset(const bundle_t bundle, const uint32_t index) @@ -428,6 +430,11 @@ static bool esp_crt_check_bundle(const uint8_t* const x509_bundle, const size_t return false; } + if (unlikely(num_certs == 0)) { + // No certificates: the loops below compute num_certs - 1, which would underflow. + return false; + } + // Check all offsets for consistency with certificate data for (uint32_t i = 0; i < num_certs - 1; ++i) { const uint32_t off = offsets[i]; @@ -440,6 +447,17 @@ static bool esp_crt_check_bundle(const uint8_t* const x509_bundle, const size_t } } + // The loop above stops at num_certs - 1, so the final certificate's extent is never + // validated; check it explicitly so its key data cannot run past the bundle (CWE-125). + const uint32_t last_off = offsets[num_certs - 1]; + if (unlikely(last_off >= bundle_size)) { + return false; + } + const uint32_t last_len = esp_crt_get_len(x509_bundle + last_off); + if (unlikely((uint64_t)last_off + last_len > bundle_size)) { + return false; + } + // All checks passed. return true; } diff --git a/components/mbedtls/port/aes/esp_aes.c b/components/mbedtls/port/aes/esp_aes.c index 39a803d08b4..826882c06b2 100644 --- a/components/mbedtls/port/aes/esp_aes.c +++ b/components/mbedtls/port/aes/esp_aes.c @@ -6,7 +6,7 @@ * * SPDX-License-Identifier: Apache-2.0 * - * SPDX-FileContributor: 2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileContributor: 2025-2026 Espressif Systems (Shanghai) CO LTD */ /* * The AES block cipher was designed by Vincent Rijmen and Joan Daemen. @@ -459,6 +459,15 @@ int esp_aes_crypt_cfb128(esp_aes_context *ctx, n = *iv_off; + /* iv[] is a fixed 16-byte buffer and n indexes it directly (before the modulo-16 update), + * so a caller-supplied *iv_off > 15 -- attacker-controlled via the TEE secure service -- + * is an out-of-bounds read/write of iv[] in TEE context (CWE-787 / CWE-125). Bound it here, + * matching the guard already present in esp_aes_crypt_ofb(). */ + if (n >= AES_BLOCK_BYTES) { + ESP_LOGE(TAG, "IV offset out of bounds"); + return MBEDTLS_ERR_AES_BAD_INPUT_DATA; + } + #if SOC_AES_SUPPORT_DMA #if CONFIG_MBEDTLS_AES_HW_SMALL_DATA_LEN_OPTIM if (length > AES_DMA_MODE_THRESHOLD) { @@ -583,6 +592,10 @@ int esp_aes_crypt_ofb(esp_aes_context *ctx, } n = *iv_off; + if (n >= AES_BLOCK_BYTES) { + ESP_LOGE(TAG, "IV offset out of bounds"); + return MBEDTLS_ERR_AES_BAD_INPUT_DATA; + } #if SOC_AES_SUPPORT_DMA #if CONFIG_MBEDTLS_AES_HW_SMALL_DATA_LEN_OPTIM @@ -687,6 +700,10 @@ int esp_aes_crypt_ctr(esp_aes_context *ctx, } n = *nc_off; + if (n >= AES_BLOCK_BYTES) { + ESP_LOGE(TAG, "IV offset out of bounds"); + return MBEDTLS_ERR_AES_BAD_INPUT_DATA; + } #if SOC_AES_SUPPORT_DMA #if CONFIG_MBEDTLS_AES_HW_SMALL_DATA_LEN_OPTIM diff --git a/components/mbedtls/port/aes/esp_aes_gcm.c b/components/mbedtls/port/aes/esp_aes_gcm.c index e915259ea8f..9850969eab8 100644 --- a/components/mbedtls/port/aes/esp_aes_gcm.c +++ b/components/mbedtls/port/aes/esp_aes_gcm.c @@ -471,6 +471,14 @@ int esp_aes_gcm_update( esp_gcm_context *ctx, return PSA_ERROR_INVALID_ARGUMENT; } + /* Honor the documented contract: the output buffer must hold input_length bytes, which are + * written unconditionally below; without this check an undersized buffer overflows (CWE-20 + * -> CWE-787). MBEDTLS_ERR_GCM_BAD_INPUT is #defined to PSA_ERROR_INVALID_ARGUMENT. */ + if ( output_size < input_length ) { + ESP_LOGE(TAG, "Output buffer too small"); + return PSA_ERROR_INVALID_ARGUMENT; + } + if ( output > input && (size_t) ( output - input ) < input_length ) { return ( PSA_ERROR_INVALID_ARGUMENT ); } @@ -595,7 +603,7 @@ static int esp_aes_gcm_crypt_and_tag_partial_hw( esp_gcm_context *ctx, return ( ret ); } - if ( ( ret = esp_aes_gcm_update( ctx, input, length, output, 0, &olen ) ) != 0 ) { + if ( ( ret = esp_aes_gcm_update( ctx, input, length, output, length, &olen ) ) != 0 ) { return ( ret ); } @@ -622,6 +630,12 @@ int esp_aes_gcm_crypt_and_tag( esp_gcm_context *ctx, ESP_LOGE(TAG, "No AES context supplied"); return PSA_ERROR_INVALID_ARGUMENT; } + /* GCM tags are 4..16 bytes. Validate here so the hardware path also rejects an invalid + * tag_len (the software path enforces this in esp_aes_gcm_finish()); otherwise the HAL tag + * read would be driven with an out-of-range length (CWE-125 / CWE-787). */ + if ( tag_len < 4 || tag_len > 16 ) { + return PSA_ERROR_INVALID_ARGUMENT; + } #if CONFIG_MBEDTLS_HARDWARE_GCM int ret; size_t remainder_bit; @@ -717,6 +731,13 @@ int esp_aes_gcm_auth_decrypt( esp_gcm_context *ctx, size_t i; int diff; + /* Validate tag_len before use: a zero tag_len makes the constant-time comparison loop + * below run zero iterations, so diff stays 0 and any forged ciphertext is accepted as + * authentic (CWE-347). Enforce the same 4..16 range as esp_aes_gcm_finish(). */ + if ( tag_len > 16 || tag_len < 4 ) { + return PSA_ERROR_INVALID_ARGUMENT; + } + if ( ( ret = esp_aes_gcm_crypt_and_tag( ctx, ESP_AES_DECRYPT, length, iv, iv_len, aad, aad_len, input, output, tag_len, check_tag ) ) != 0 ) { diff --git a/components/mbedtls/port/ecc/esp_ecc.c b/components/mbedtls/port/ecc/esp_ecc.c index 43060022fe5..59a558fd2e7 100644 --- a/components/mbedtls/port/ecc/esp_ecc.c +++ b/components/mbedtls/port/ecc/esp_ecc.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -33,6 +33,17 @@ int esp_ecc_point_multiply(const ecc_point_t *point, const uint8_t *scalar, ecc_ uint16_t len = point->len; ecc_mode_t work_mode = verify_first ? ECC_MODE_VERIFY_THEN_POINT_MUL : ECC_MODE_POINT_MUL; + /* len is used as the HW read/write byte count for the fixed-size ecc_point_t buffers; + * reject any value that is not a supported curve length before touching hardware. On the + * TEE secure-service path this field is attacker-controlled (CWE-20 -> OOB read/write). */ + if (len != P192_LEN && len != P256_LEN +#if SOC_ECC_SUPPORT_CURVE_P384 + && len != P384_LEN +#endif + ) { + return -1; + } + esp_ecc_acquire_hardware(); ecc_hal_write_mul_param(scalar, point->x, point->y, len); @@ -65,6 +76,18 @@ int esp_ecc_point_verify(const ecc_point_t *point) { int result; + /* point->len drives a fixed-stride MMIO write loop in the HAL; an unvalidated oversized + * value (attacker-controlled via the TEE secure service) walks past the ECC register block + * and can reach other peripheral registers (CWE-787). Reject non-curve lengths up front and + * return 0 (point not verified) -- the fail-safe value for this routine. */ + if (point->len != P192_LEN && point->len != P256_LEN +#if SOC_ECC_SUPPORT_CURVE_P384 + && point->len != P384_LEN +#endif + ) { + return 0; + } + esp_ecc_acquire_hardware(); ecc_hal_write_verify_param(point->x, point->y, point->len); ecc_hal_set_mode(ECC_MODE_VERIFY);