Conversation
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
🤖 Augment PR SummarySummary: Upgrades the vendored Mbed TLS dependency from v3.6.5 to v3.6.6 and updates the build integration accordingly. Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
| key_buffer_size = PSA_EXPORT_KEY_OUTPUT_SIZE(old_type, old_bits); | ||
| key_buffer = mbedtls_calloc(1, key_buffer_size); | ||
| if (key_buffer == NULL) { | ||
| return MBEDTLS_ERR_PK_ALLOC_FAILED; |
There was a problem hiding this comment.
vendor/mbedtls/library/pk.c:632: export_import_into_psa() returns MBEDTLS_ERR_PK_ALLOC_FAILED even though the function return type is psa_status_t, which mixes error-code domains and can confuse the PSA_PK_TO_MBEDTLS_ERR() mapping at the call site. Consider returning an appropriate PSA status for allocation failure so callers handle it consistently.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| /* mbedtls_rsa_write_key() uses the same format as PSA export, which | ||
| * actually calls it under the hood, so we can use the PSA size macro. */ | ||
| const size_t buf_size = PSA_KEY_EXPORT_RSA_KEY_PAIR_MAX_SIZE(key_bits); | ||
| unsigned char *buf = mbedtls_calloc(1, buf_size); |
There was a problem hiding this comment.
vendor/mbedtls/library/pk_wrap.c:302: buf from mbedtls_calloc() is used immediately in pointer arithmetic (buf + buf_size) without a NULL check, which will crash on allocation failure. Consider handling OOM here before using buf.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| unsigned char *p = buf + buf_size; | ||
| key_len = mbedtls_rsa_write_key(rsa, buf, &p); | ||
| if (key_len <= 0) { | ||
| return MBEDTLS_ERR_PK_BAD_INPUT_DATA; |
There was a problem hiding this comment.
vendor/mbedtls/library/pk_wrap.c:307: on mbedtls_rsa_write_key() failure this path returns directly and skips cleanup:, so the heap buffer holding private key material is leaked and not zeroized. Consider routing this failure through cleanup: so mbedtls_zeroize_and_free() always runs.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| /* This is a (grand...)child of the original process, but | ||
| * we inherited the RNG state from our parent. We must reseed! */ | ||
| #if defined(MBEDTLS_THREADING_C) | ||
| mbedtls_mutex_lock(&mbedtls_threading_psa_rngdata_mutex); |
There was a problem hiding this comment.
vendor/mbedtls/library/psa_crypto_random.c:147: the return value of mbedtls_mutex_lock() is ignored, but mbedtls_mutex_unlock() is called unconditionally, which could result in unlocking an unheld mutex and data races if locking fails. Consider checking the lock result and failing with a PSA error if the mutex can’t be acquired.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
Signed-off-by: Juan Cruz Viotti jv@jviotti.com