mirror of
https://github.com/espressif/esp-idf.git
synced 2026-10-01 18:50:34 +03:00
fix(mbedtls): validate crypto input lengths (TEE OOB, auth-bypass, overflows)
This commit is contained in:
@@ -99,9 +99,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)
|
||||
@@ -305,6 +307,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 = esp_crt_get_cert_offset(x509_bundle, i);
|
||||
@@ -318,6 +325,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 = esp_crt_get_cert_offset(x509_bundle, 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;
|
||||
}
|
||||
|
||||
@@ -358,6 +358,16 @@ int esp_aes_crypt_cfb128(esp_aes_context *ctx,
|
||||
|
||||
int c;
|
||||
size_t n = *iv_off;
|
||||
|
||||
/* iv[] is a fixed AES_BLOCK_BYTES buffer and n indexes it directly (before the modulo update),
|
||||
* so a caller-supplied *iv_off >= AES_BLOCK_BYTES -- 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 (before acquiring the AES hardware) matching esp_aes_crypt_ofb(). */
|
||||
if (n >= AES_BLOCK_BYTES) {
|
||||
ESP_LOGE(TAG, "IV offset out of bounds");
|
||||
return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
|
||||
}
|
||||
|
||||
esp_aes_acquire_hardware();
|
||||
ctx->key_in_hardware = 0;
|
||||
ctx->key_in_hardware = aes_hal_setkey(ctx->key, ctx->key_bytes, ESP_AES_ENCRYPT);
|
||||
@@ -491,6 +501,17 @@ int esp_aes_crypt_ctr(esp_aes_context *ctx,
|
||||
}
|
||||
|
||||
size_t n = *nc_off;
|
||||
|
||||
/* stream_block[] is a fixed AES_BLOCK_BYTES buffer and n indexes it directly (before the
|
||||
* modulo update), so a caller-supplied *nc_off >= AES_BLOCK_BYTES -- attacker-controlled via
|
||||
* the TEE secure service -- is an out-of-bounds read of stream_block[] in TEE context
|
||||
* (CWE-125), leaking adjacent memory into the output. Bound it here, matching
|
||||
* esp_aes_crypt_cfb128() and esp_aes_crypt_ofb(). */
|
||||
if (n >= AES_BLOCK_BYTES) {
|
||||
ESP_LOGE(TAG, "Nonce offset out of bounds");
|
||||
return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
|
||||
}
|
||||
|
||||
if (!valid_key_length(ctx)) {
|
||||
return MBEDTLS_ERR_AES_INVALID_KEY_LENGTH;
|
||||
}
|
||||
@@ -556,7 +577,7 @@ int esp_aes_crypt_ofb(esp_aes_context *ctx,
|
||||
|
||||
n = *iv_off;
|
||||
|
||||
if (n > 15) {
|
||||
if (n >= AES_BLOCK_BYTES) {
|
||||
return (MBEDTLS_ERR_AES_BAD_INPUT_DATA);
|
||||
}
|
||||
|
||||
|
||||
@@ -347,6 +347,15 @@ int esp_aes_crypt_cfb128(esp_aes_context *ctx,
|
||||
|
||||
n = *iv_off;
|
||||
|
||||
/* iv[] is a fixed AES_BLOCK_BYTES buffer and n indexes it directly (before the modulo update),
|
||||
* so a caller-supplied *iv_off >= AES_BLOCK_BYTES -- 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. */
|
||||
if (n >= AES_BLOCK_BYTES) {
|
||||
ESP_LOGE(TAG, "IV offset out of bounds");
|
||||
return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
|
||||
}
|
||||
|
||||
/* First process the *iv_off bytes
|
||||
* which are pending from the previous call to this API
|
||||
*/
|
||||
@@ -492,6 +501,16 @@ int esp_aes_crypt_ctr(esp_aes_context *ctx,
|
||||
|
||||
n = *nc_off;
|
||||
|
||||
/* stream_block[] is a fixed AES_BLOCK_BYTES buffer and n indexes it directly (before the
|
||||
* modulo update), so a caller-supplied *nc_off >= AES_BLOCK_BYTES -- attacker-controlled via
|
||||
* the TEE secure service -- is an out-of-bounds read of stream_block[] in TEE context
|
||||
* (CWE-125), leaking adjacent memory into the output. Bound it here, matching
|
||||
* esp_aes_crypt_cfb128() and esp_aes_crypt_ofb(). */
|
||||
if (n >= AES_BLOCK_BYTES) {
|
||||
ESP_LOGE(TAG, "Nonce offset out of bounds");
|
||||
return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
|
||||
}
|
||||
|
||||
if (!valid_key_length(ctx)) {
|
||||
return MBEDTLS_ERR_AES_INVALID_KEY_LENGTH;
|
||||
}
|
||||
|
||||
@@ -518,6 +518,14 @@ int esp_aes_gcm_update( esp_gcm_context *ctx,
|
||||
return MBEDTLS_ERR_GCM_BAD_INPUT;
|
||||
}
|
||||
|
||||
/* 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 the bad-input error code used throughout this file. */
|
||||
if ( output_size < input_length ) {
|
||||
ESP_LOGE(TAG, "Output buffer too small");
|
||||
return MBEDTLS_ERR_GCM_BAD_INPUT;
|
||||
}
|
||||
|
||||
if ( output > input && (size_t) ( output - input ) < input_length ) {
|
||||
return ( MBEDTLS_ERR_GCM_BAD_INPUT );
|
||||
}
|
||||
@@ -638,7 +646,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 );
|
||||
}
|
||||
|
||||
@@ -671,6 +679,12 @@ int esp_aes_gcm_crypt_and_tag( esp_gcm_context *ctx,
|
||||
return mbedtls_gcm_crypt_and_tag_soft(ctx->ctx_soft, mode, length, iv, iv_len, aad, aad_len, input, output, tag_len, tag);
|
||||
}
|
||||
#endif
|
||||
/* 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 MBEDTLS_ERR_GCM_BAD_INPUT;
|
||||
}
|
||||
#if CONFIG_MBEDTLS_HARDWARE_GCM
|
||||
int ret;
|
||||
size_t remainder_bit;
|
||||
@@ -771,6 +785,14 @@ 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); an oversized tag_len also over-reads the 16-byte check_tag
|
||||
* (CWE-125). Enforce the same 4..16 range as esp_aes_gcm_finish(). */
|
||||
if ( tag_len > 16 || tag_len < 4 ) {
|
||||
return MBEDTLS_ERR_GCM_BAD_INPUT;
|
||||
}
|
||||
|
||||
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 ) {
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user