crypto,tls: do not ignore BN_get_word error

tniessen · aduh95 · commit 88011a36898b · 2026-06-20T16:58:11.000+02:00
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>
diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc
@@ -408,12 +408,16 @@ bool BignumPointer::setWord(unsigned long w) {  // NOLINT(runtime/int)
   return BN_set_word(bn_.get(), w) == 1;
 }
 
-unsigned long BignumPointer::GetWord(const BIGNUM* bn) {  // NOLINT(runtime/int)
-  return BN_get_word(bn);
+std::optional<unsigned long> BignumPointer::GetWord(  // NOLINT(runtime/int)
+    const BIGNUM* bn) {
+  BN_ULONG ret = BN_get_word(bn);
+  if (ret == static_cast<BN_ULONG>(-1)) return std::nullopt;
+  return ret;
 }
 
-unsigned long BignumPointer::getWord() const {  // NOLINT(runtime/int)
-  if (!bn_) return 0;
+std::optional<unsigned long> BignumPointer::getWord()  // NOLINT(runtime/int)
+    const {
+  if (!bn_) return std::nullopt;
   return GetWord(bn_.get());
 }
 
diff --git a/deps/ncrypto/ncrypto.h b/deps/ncrypto/ncrypto.h
@@ -743,7 +743,7 @@ class BignumPointer final {
   bool isOne() const;
 
   bool setWord(unsigned long w);  // NOLINT(runtime/int)
-  unsigned long getWord() const;  // NOLINT(runtime/int)
+  std::optional<unsigned long> getWord() const;  // NOLINT(runtime/int)
 
   size_t byteLength() const;
 
@@ -782,7 +782,8 @@ class BignumPointer final {
                                  size_t size);
   static int GetBitCount(const BIGNUM* bn);
   static int GetByteCount(const BIGNUM* bn);
-  static unsigned long GetWord(const BIGNUM* bn);  // NOLINT(runtime/int)
+  static std::optional<unsigned long> GetWord(  // NOLINT(runtime/int)
+      const BIGNUM* bn);
   static const BIGNUM* One();
 
   BignumPointer clone();
diff --git a/src/crypto/crypto_aes.cc b/src/crypto/crypto_aes.cc
@@ -384,7 +384,9 @@ WebCryptoCipherStatus AES_CTR_Cipher(Environment* env,
     return status;
   }
 
-  BN_ULONG input_size_part1 = remaining_until_reset.getWord() * kAesBlockSize;
+  std::optional<BN_ULONG> remaining_blocks = remaining_until_reset.getWord();
+  CHECK(remaining_blocks.has_value());
+  BN_ULONG input_size_part1 = remaining_blocks.value() * kAesBlockSize;
 
   // Encrypt the first part...
   auto status =
diff --git a/src/crypto/crypto_x509.cc b/src/crypto/crypto_x509.cc
@@ -44,6 +44,7 @@ using v8::Local;
 using v8::LocalVector;
 using v8::MaybeLocal;
 using v8::NewStringType;
+using v8::Null;
 using v8::Object;
 using v8::String;
 using v8::Uint32;
@@ -688,11 +689,12 @@ MaybeLocal<Value> GetModulusString(Environment* env, const BIGNUM* n) {
 }
 
 MaybeLocal<Value> GetExponentString(Environment* env, const BIGNUM* e) {
-  uint64_t exponent_word = static_cast<uint64_t>(BignumPointer::GetWord(e));
+  auto exponent_word = BignumPointer::GetWord(e);
+  if (!exponent_word) return Null(env->isolate());
   auto bio = BIOPointer::NewMem();
   if (!bio) [[unlikely]]
     return {};
-  BIO_printf(bio.get(), "0x%" PRIx64, exponent_word);
+  BIO_printf(bio.get(), "0x%" PRIx64, static_cast<uint64_t>(*exponent_word));
   return ToV8Value(env->context(), bio);
 }
 

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
`@@ -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 =`