Skip to content

Commit 88011a3

Browse files
tniessenaduh95
authored andcommitted
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 82dd7dd commit 88011a3

4 files changed

Lines changed: 18 additions & 9 deletions

File tree

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -408,12 +408,16 @@ bool BignumPointer::setWord(unsigned long w) { // NOLINT(runtime/int)
408408
returnBN_set_word(bn_.get(), w) == 1;
409409
}
410410

411-
unsignedlongBignumPointer::GetWord(constBIGNUM* bn) { // NOLINT(runtime/int)
412-
returnBN_get_word(bn);
411+
std::optional<unsignedlong> BignumPointer::GetWord( // NOLINT(runtime/int)
412+
constBIGNUM* bn) {
413+
BN_ULONG ret = BN_get_word(bn);
414+
if (ret == static_cast<BN_ULONG>(-1)) return std::nullopt;
415+
return ret;
413416
}
414417

415-
unsignedlongBignumPointer::getWord() const { // NOLINT(runtime/int)
416-
if (!bn_) return0;
418+
std::optional<unsignedlong> BignumPointer::getWord() // NOLINT(runtime/int)
419+
const {
420+
if (!bn_) return std::nullopt;
417421
returnGetWord(bn_.get());
418422
}
419423

‎deps/ncrypto/ncrypto.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -743,7 +743,7 @@ class BignumPointer final {
743743
boolisOne() const;
744744

745745
boolsetWord(unsignedlong w); // NOLINT(runtime/int)
746-
unsignedlonggetWord() const; // NOLINT(runtime/int)
746+
std::optional<unsignedlong>getWord() const; // NOLINT(runtime/int)
747747

748748
size_tbyteLength() const;
749749

@@ -782,7 +782,8 @@ class BignumPointer final {
782782
size_t size);
783783
staticintGetBitCount(constBIGNUM* bn);
784784
staticintGetByteCount(constBIGNUM* bn);
785-
staticunsignedlongGetWord(constBIGNUM* bn); // NOLINT(runtime/int)
785+
static std::optional<unsignedlong> GetWord( // NOLINT(runtime/int)
786+
constBIGNUM* bn);
786787
staticconstBIGNUM* One();
787788

788789
BignumPointer clone();

‎src/crypto/crypto_aes.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,9 @@ WebCryptoCipherStatus AES_CTR_Cipher(Environment* env,
384384
return status;
385385
}
386386

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;
388390

389391
// Encrypt the first part...
390392
auto status =

‎src/crypto/crypto_x509.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ using v8::Local;
4444
using v8::LocalVector;
4545
using v8::MaybeLocal;
4646
using v8::NewStringType;
47+
using v8::Null;
4748
using v8::Object;
4849
using v8::String;
4950
using v8::Uint32;
@@ -688,11 +689,12 @@ MaybeLocal<Value> GetModulusString(Environment* env, const BIGNUM* n) {
688689
}
689690

690691
MaybeLocal<Value> GetExponentString(Environment* env, constBIGNUM* e) {
691-
uint64_t exponent_word = static_cast<uint64_t>(BignumPointer::GetWord(e));
692+
auto exponent_word = BignumPointer::GetWord(e);
693+
if (!exponent_word) returnNull(env->isolate());
692694
auto bio = BIOPointer::NewMem();
693695
if (!bio) [[unlikely]]
694696
return {};
695-
BIO_printf(bio.get(), "0x%" PRIx64, exponent_word);
697+
BIO_printf(bio.get(), "0x%" PRIx64, static_cast<uint64_t>(*exponent_word));
696698
returnToV8Value(env->context(), bio);
697699
}
698700

0 commit comments

Comments
 (0)