From 1050151d2ed8ecfbf59717857620599d7391d997 Mon Sep 17 00:00:00 2001 From: Evgeny Margolis Date: Thu, 19 May 2022 20:51:23 -0700 Subject: [PATCH] Fix Signature R and S Elements Size Checks in ECDSA_sign_hash(). (#18629) The implementation was erroneously using || instead of &&. The OpenSSL implementation updated the verify that "size <= kP256_FE_Length" instead of "==". In the OpenSSL implementation the current check was never performed because the first condition was always true "(r != nullptr)". --- src/crypto/CHIPCryptoPALOpenSSL.cpp | 5 ++++- src/crypto/CHIPCryptoPALmbedTLS.cpp | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/crypto/CHIPCryptoPALOpenSSL.cpp b/src/crypto/CHIPCryptoPALOpenSSL.cpp index b03c3564a8d964..4c66d912f1f779 100644 --- a/src/crypto/CHIPCryptoPALOpenSSL.cpp +++ b/src/crypto/CHIPCryptoPALOpenSSL.cpp @@ -645,7 +645,10 @@ CHIP_ERROR P256Keypair::ECDSA_sign_hash(const uint8_t * hash, const size_t hash_ VerifyOrExit(sig != nullptr, error = CHIP_ERROR_INTERNAL); ECDSA_SIG_get0(sig, &r, &s); - VerifyOrExit((r != nullptr) || (s != nullptr) || (BN_num_bytes(r) == kP256_FE_Length) || (BN_num_bytes(s) == kP256_FE_Length), + VerifyOrExit((r != nullptr) && (s != nullptr), error = CHIP_ERROR_INTERNAL); + VerifyOrExit(CanCastTo(BN_num_bytes(r)) && CanCastTo(BN_num_bytes(s)), error = CHIP_ERROR_INTERNAL); + VerifyOrExit((static_cast(BN_num_bytes(r)) <= kP256_FE_Length) && + (static_cast(BN_num_bytes(s)) <= kP256_FE_Length), error = CHIP_ERROR_INTERNAL); // Concatenate r and s to output. Sizes were checked above. diff --git a/src/crypto/CHIPCryptoPALmbedTLS.cpp b/src/crypto/CHIPCryptoPALmbedTLS.cpp index 56a33edb85a5d9..8b63966b1bce21 100644 --- a/src/crypto/CHIPCryptoPALmbedTLS.cpp +++ b/src/crypto/CHIPCryptoPALmbedTLS.cpp @@ -545,7 +545,7 @@ CHIP_ERROR P256Keypair::ECDSA_sign_hash(const uint8_t * hash, const size_t hash_ VerifyOrExit(result == 0, error = CHIP_ERROR_INTERNAL); - VerifyOrExit((mbedtls_mpi_size(&r) <= kP256_FE_Length) || (mbedtls_mpi_size(&s) <= kP256_FE_Length), + VerifyOrExit((mbedtls_mpi_size(&r) <= kP256_FE_Length) && (mbedtls_mpi_size(&s) <= kP256_FE_Length), error = CHIP_ERROR_INTERNAL); // Concatenate r and s to output. Sizes were checked above.