crypto: fix regression in Hash, Hmac, SignBase, PublicKeyCipher

nodejs#31406 introduced a regression in
`Hash`, `Hmac`, `SignBase`, and `PublicKeyCipher`

Signed-off-by: James M Snell <[email protected]>

src/node_crypto.cc

@@ -3983,9 +3983,9 @@ void CipherBase::SetAAD(const FunctionCallbackInfo<Value>& args) {
 }
 
 CipherBase::UpdateResult CipherBase::Update(const char* data,
-                                            int len,
+                                            size_t len,
                                             AllocatedBuffer* out) {
-  if (!ctx_)
+  if (!ctx_ || len > INT_MAX)
     return kErrorState;
   MarkPopErrorOnReturn mark_pop_error_on_return;
 
@@ -4039,11 +4039,15 @@ void CipherBase::Update(const FunctionCallbackInfo<Value>& args) {
                               const FunctionCallbackInfo<Value>& args,
                               const char* data, size_t size) {
     AllocatedBuffer out;
+    Environment* env = Environment::GetCurrent(args);
+
+    if (size > INT_MAX)
+      return THROW_ERR_OUT_OF_RANGE(env, "data is too long");
+
     UpdateResult r = cipher->Update(data, size, &out);
 
     if (r != kSuccess) {
       if (r == kErrorState) {
-        Environment* env = Environment::GetCurrent(args);
         ThrowCryptoError(env, ERR_get_error(),
                          "Trying to add data in unsupported state");
       }
@@ -4205,7 +4209,7 @@ void Hmac::HmacInit(const FunctionCallbackInfo<Value>& args) {
 }
 
 
-bool Hmac::HmacUpdate(const char* data, int len) {
+bool Hmac::HmacUpdate(const char* data, size_t len) {
   if (!ctx_)
     return false;
   int r = HMAC_Update(ctx_.get(),
@@ -4341,7 +4345,7 @@ bool Hash::HashInit(const EVP_MD* md, Maybe<unsigned int> xof_md_len) {
 }
 
 
-bool Hash::HashUpdate(const char* data, int len) {
+bool Hash::HashUpdate(const char* data, size_t len) {
   if (!mdctx_)
     return false;
   EVP_DigestUpdate(mdctx_.get(), data, len);
@@ -4443,7 +4447,7 @@ SignBase::Error SignBase::Init(const char* sign_type) {
 }
 
 
-SignBase::Error SignBase::Update(const char* data, int len) {
+SignBase::Error SignBase::Update(const char* data, size_t len) {
   if (mdctx_ == nullptr)
     return kSignNotInitialised;
   if (!EVP_DigestUpdate(mdctx_.get(), data, len))
@@ -5050,7 +5054,7 @@ bool PublicKeyCipher::Cipher(Environment* env,
                              const void* oaep_label,
                              size_t oaep_label_len,
                              const unsigned char* data,
-                             int len,
+                             size_t len,
                              AllocatedBuffer* out) {
   EVPKeyCtxPointer ctx(EVP_PKEY_CTX_new(pkey.get(), nullptr));
   if (!ctx)
@@ -5129,6 +5133,19 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
     oaep_label.Read(args[offset + 3].As<ArrayBufferView>());
   }
 
+  // For rsa ciphers, the maximum size for buf and oaep_label is 2**31-1.
+  // This is because internally, for rsa ciphers, OpenSSL downcasts the
+  // input lengths to int rather than size_t, leading to segfaults if any
+  // value larger than 2**31-1 is used. Other ciphers may not have the
+  // same limitation, but given that this function is written largely to
+  // assume RSA in every other way, and that's the most likely use, we'll
+  // enforce the limit here.
+  if (oaep_label.length() > INT_MAX)
+    return THROW_ERR_OUT_OF_RANGE(env, "oaepLabel is too long");
+
+  if (buf.length() > INT_MAX)
+    return THROW_ERR_OUT_OF_RANGE(env, "buffer is too long");
+
   AllocatedBuffer out;
 
   bool r = Cipher<operation, EVP_PKEY_cipher_init, EVP_PKEY_cipher>(

src/node_crypto.h

@@ -571,7 +571,7 @@ class CipherBase : public BaseObject {
   bool InitAuthenticated(const char* cipher_type, int iv_len,
                          unsigned int auth_tag_len);
   bool CheckCCMMessageLength(int message_len);
-  UpdateResult Update(const char* data, int len, AllocatedBuffer* out);
+  UpdateResult Update(const char* data, size_t len, AllocatedBuffer* out);
   bool Final(AllocatedBuffer* out);
   bool SetAutoPadding(bool auto_padding);
 
@@ -613,7 +613,7 @@ class Hmac : public BaseObject {
 
  protected:
   void HmacInit(const char* hash_type, const char* key, int key_len);
-  bool HmacUpdate(const char* data, int len);
+  bool HmacUpdate(const char* data, size_t len);
 
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void HmacInit(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -638,7 +638,7 @@ class Hash final : public BaseObject {
   SET_SELF_SIZE(Hash)
 
   bool HashInit(const EVP_MD* md, v8::Maybe<unsigned int> xof_md_len);
-  bool HashUpdate(const char* data, int len);
+  bool HashUpdate(const char* data, size_t len);
 
  protected:
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -670,7 +670,7 @@ class SignBase : public BaseObject {
   SignBase(Environment* env, v8::Local<v8::Object> wrap);
 
   Error Init(const char* sign_type);
-  Error Update(const char* data, int len);
+  Error Update(const char* data, size_t len);
 
   // TODO(joyeecheung): track the memory used by OpenSSL types
   SET_NO_MEMORY_INFO()
@@ -757,7 +757,7 @@ class PublicKeyCipher {
                      const void* oaep_label,
                      size_t oaep_label_size,
                      const unsigned char* data,
-                     int len,
+                     size_t len,
                      AllocatedBuffer* out);
 
   template <Operation operation,

test/parallel/test-crypto-hash-hmac-regression.js

@@ -0,0 +1,72 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+if (!common.enoughTestMem)
+  common.skip('skip on low memory machines');
+
+const assert = require('assert');
+
+const {
+  Buffer
+} = require('buffer');
+
+const {
+  createHash,
+  createHmac,
+  createSign,
+  createCipheriv,
+  randomBytes,
+  generateKeyPairSync,
+  publicEncrypt,
+  publicDecrypt,
+} = require('crypto');
+
+let kData;
+try {
+  kData = Buffer.alloc(2 ** 31 + 1);
+} catch {
+  // If the Buffer could not be allocated for some reason,
+  // just skip the test. There are some systems in CI that
+  // simply cannot allocated a large enough buffer.
+  common.skip('skip on low memory machines');
+}
+
+// https://github.com/nodejs/node/pull/31406 changed the maximum value
+// of buffer.kMaxLength but Hash, Hmac, and SignBase, CipherBase update
+// expected int for the size, causing both of the following to segfault.
+// Both update operations have been updated to expected size size_t.
+
+// Closely related is the fact that publicEncrypt max input size is
+// 2**31-1 due to limitations in OpenSSL.
+
+createHash('sha512').update(kData);
+
+createHmac('sha512', 'a secret').update(kData);
+
+createSign('sha512').update(kData);
+
+const { publicKey, privateKey } = generateKeyPairSync('rsa', {
+  modulusLength: 1024,
+  publicExponent: 3
+});
+
+assert.throws(() => publicEncrypt(publicKey, kData), {
+  code: 'ERR_OUT_OF_RANGE'
+});
+
+assert.throws(() => publicDecrypt(privateKey, kData), {
+  code: 'ERR_OUT_OF_RANGE'
+});
+
+{
+  const cipher = createCipheriv(
+    'aes-192-cbc',
+    'key'.repeat(8),
+    randomBytes(16));
+
+  assert.throws(() => cipher.update(kData), {
+    code: 'ERR_OUT_OF_RANGE'
+  });
+}