Commit 329e549
crypto,tls: do not ignore BN_get_word error
This changes `BignumPointer::GetWord` such that it does not hide errors
from the caller. In the context of RSA keys within X.509 certificates,
we should eventually compute the public exponent correctly regardless of
its size. This patch, however, is designed to be a minimal change that
prevents callers from using erroneous return values of `BN_get_word`.
Signed-off-by: Tobias Nießen <tniessen@tnie.de>
PR-URL: #63895
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>1 parent 0ae1552 commit 329e549
4 files changed
Lines changed: 18 additions & 9 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -408,12 +408,16 @@ bool BignumPointer::setWord(unsigned long w) { // NOLINT(runtime/int) | |||
| 408 | 408 | return BN_set_word(bn_.get(), w) == 1; | |
| 409 | 409 | } | |
| 410 | 410 | ||
| 411 | - unsigned long BignumPointer::GetWord(const BIGNUM* bn) { // NOLINT(runtime/int) | ||
| 412 | - return BN_get_word(bn); | ||
| 411 | + std::optional<unsigned long> BignumPointer::GetWord( // NOLINT(runtime/int) | ||
| 412 | + const BIGNUM* bn) { | ||
| 413 | + BN_ULONG ret = BN_get_word(bn); | ||
| 414 | + if (ret == static_cast<BN_ULONG>(-1)) return std::nullopt; | ||
| 415 | + return ret; | ||
| 413 | 416 | } | |
| 414 | 417 | ||
| 415 | - unsigned long BignumPointer::getWord() const { // NOLINT(runtime/int) | ||
| 416 | - if (!bn_) return 0; | ||
| 418 | + std::optional<unsigned long> BignumPointer::getWord() // NOLINT(runtime/int) | ||
| 419 | + const { | ||
| 420 | + if (!bn_) return std::nullopt; | ||
| 417 | 421 | return GetWord(bn_.get()); | |
| 418 | 422 | } | |
| 419 | 423 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -743,7 +743,7 @@ class BignumPointer final { | |||
| 743 | 743 | bool isOne() const; | |
| 744 | 744 | ||
| 745 | 745 | bool setWord(unsigned long w); // NOLINT(runtime/int) | |
| 746 | - unsigned long getWord() const; // NOLINT(runtime/int) | ||
| 746 | + std::optional<unsigned long> getWord() const; // NOLINT(runtime/int) | ||
| 747 | 747 | ||
| 748 | 748 | size_t byteLength() const; | |
| 749 | 749 | ||
@@ -782,7 +782,8 @@ class BignumPointer final { | |||
| 782 | 782 | size_t size); | |
| 783 | 783 | static int GetBitCount(const BIGNUM* bn); | |
| 784 | 784 | static int GetByteCount(const BIGNUM* bn); | |
| 785 | - static unsigned long GetWord(const BIGNUM* bn); // NOLINT(runtime/int) | ||
| 785 | + static std::optional<unsigned long> GetWord( // NOLINT(runtime/int) | ||
| 786 | + const BIGNUM* bn); | ||
| 786 | 787 | static const BIGNUM* One(); | |
| 787 | 788 | ||
| 788 | 789 | BignumPointer clone(); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -384,7 +384,9 @@ WebCryptoCipherStatus AES_CTR_Cipher(Environment* env, | |||
| 384 | 384 | return status; | |
| 385 | 385 | } | |
| 386 | 386 | ||
| 387 | - BN_ULONG input_size_part1 = remaining_until_reset.getWord() * kAesBlockSize; | ||
| 387 | + std::optional<BN_ULONG> remaining_blocks = remaining_until_reset.getWord(); | ||
| 388 | + CHECK(remaining_blocks.has_value()); | ||
| 389 | + BN_ULONG input_size_part1 = remaining_blocks.value() * kAesBlockSize; | ||
| 388 | 390 | ||
| 389 | 391 | // Encrypt the first part... | |
| 390 | 392 | auto status = | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -44,6 +44,7 @@ using v8::Local; | |||
| 44 | 44 | using v8::LocalVector; | |
| 45 | 45 | using v8::MaybeLocal; | |
| 46 | 46 | using v8::NewStringType; | |
| 47 | + using v8::Null; | ||
| 47 | 48 | using v8::Object; | |
| 48 | 49 | using v8::String; | |
| 49 | 50 | using v8::Uint32; | |
@@ -688,11 +689,12 @@ MaybeLocal<Value> GetModulusString(Environment* env, const BIGNUM* n) { | |||
| 688 | 689 | } | |
| 689 | 690 | ||
| 690 | 691 | MaybeLocal<Value> GetExponentString(Environment* env, const BIGNUM* e) { | |
| 691 | - uint64_t exponent_word = static_cast<uint64_t>(BignumPointer::GetWord(e)); | ||
| 692 | + auto exponent_word = BignumPointer::GetWord(e); | ||
| 693 | + if (!exponent_word) return Null(env->isolate()); | ||
| 692 | 694 | auto bio = BIOPointer::NewMem(); | |
| 693 | 695 | if (!bio) [[unlikely]] | |
| 694 | 696 | return {}; | |
| 695 | - BIO_printf(bio.get(), "0x%" PRIx64, exponent_word); | ||
| 697 | + BIO_printf(bio.get(), "0x%" PRIx64, static_cast<uint64_t>(*exponent_word)); | ||
| 696 | 698 | return ToV8Value(env->context(), bio); | |
| 697 | 699 | } | |
| 698 | 700 | ||
0 commit comments