Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto: fix size_t/int regression node_crypto #35132

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 24 additions & 7 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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>(
Expand Down
10 changes: 5 additions & 5 deletions src/node_crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -757,7 +757,7 @@ class PublicKeyCipher {
const void* oaep_label,
size_t oaep_label_size,
const unsigned char* data,
int len,
size_t len,
jasnell marked this conversation as resolved.
Show resolved Hide resolved
AllocatedBuffer* out);

template <Operation operation,
Expand Down
72 changes: 72 additions & 0 deletions test/parallel/test-crypto-hash-hmac-regression.js
Original file line number Diff line number Diff line change
@@ -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'
});
}