Skip to content

Commit

Permalink
Fix Signature R and S Elements Size Checks in ECDSA_sign_hash(). (#18629
Browse files Browse the repository at this point in the history
)

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)".
  • Loading branch information
emargolis authored and pull[bot] committed Feb 20, 2024
1 parent 8526b48 commit 1050151
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 2 deletions.
5 changes: 4 additions & 1 deletion src/crypto/CHIPCryptoPALOpenSSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<size_t>(BN_num_bytes(r)) && CanCastTo<size_t>(BN_num_bytes(s)), error = CHIP_ERROR_INTERNAL);
VerifyOrExit((static_cast<size_t>(BN_num_bytes(r)) <= kP256_FE_Length) &&
(static_cast<size_t>(BN_num_bytes(s)) <= kP256_FE_Length),
error = CHIP_ERROR_INTERNAL);

// Concatenate r and s to output. Sizes were checked above.
Expand Down
2 changes: 1 addition & 1 deletion src/crypto/CHIPCryptoPALmbedTLS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 1050151

Please sign in to comment.