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.

Sign up for GitHub

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

src: fix memory leaks and refactor ByteSource #43202

Merged
merged 1 commit into from
Jun 12, 2022
Merged

src: fix memory leaks and refactor ByteSource #43202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
95 changes: 43 additions & 52 deletions src/crypto/crypto_aes.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,10 @@ WebCryptoCipherStatus AES_Cipher(
case kWebCryptoCipherDecrypt:
// If in decrypt mode, the auth tag must be set in the params.tag.
CHECK(params.tag);
if (!EVP_CIPHER_CTX_ctrl(
ctx.get(),
EVP_CTRL_AEAD_SET_TAG,
params.tag.size(),
const_cast<char*>(params.tag.get()))) {
if (!EVP_CIPHER_CTX_ctrl(ctx.get(),
EVP_CTRL_AEAD_SET_TAG,
params.tag.size(),
const_cast<char*>(params.tag.data<char>()))) {
return WebCryptoCipherStatus::FAILED;
}
break;
Expand Down Expand Up @@ -125,9 +124,7 @@ WebCryptoCipherStatus AES_Cipher(
return WebCryptoCipherStatus::FAILED;
}

char* data = MallocOpenSSL<char>(buf_len);
ByteSource buf = ByteSource::Allocated(data, buf_len);
unsigned char* ptr = reinterpret_cast<unsigned char*>(data);
ByteSource::Builder buf(buf_len);

// In some outdated version of OpenSSL (e.g.
// ubi81_sharedlibs_openssl111fips_x64) may be used in sharedlib mode, the
Expand All @@ -139,36 +136,36 @@ WebCryptoCipherStatus AES_Cipher(
// Refs: https://github.com/nodejs/node/pull/38913#issuecomment-866505244
if (in.size() == 0) {
out_len = 0;
} else if (!EVP_CipherUpdate(
ctx.get(),
ptr,
&out_len,
in.data<unsigned char>(),
in.size())) {
} else if (!EVP_CipherUpdate(ctx.get(),
buf.data<unsigned char>(),
&out_len,
in.data<unsigned char>(),
in.size())) {
return WebCryptoCipherStatus::FAILED;
}

total += out_len;
CHECK_LE(out_len, buf_len);
ptr += out_len;
out_len = EVP_CIPHER_CTX_block_size(ctx.get());
if (!EVP_CipherFinal_ex(ctx.get(), ptr, &out_len)) {
if (!EVP_CipherFinal_ex(
ctx.get(), buf.data<unsigned char>() + total, &out_len)) {
return WebCryptoCipherStatus::FAILED;
}
total += out_len;

// If using AES_GCM, grab the generated auth tag and append
// it to the end of the ciphertext.
if (cipher_mode == kWebCryptoCipherEncrypt && mode == EVP_CIPH_GCM_MODE) {
data += out_len;
if (!EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_GET_TAG, tag_len, ptr))
if (!EVP_CIPHER_CTX_ctrl(ctx.get(),
EVP_CTRL_AEAD_GET_TAG,
tag_len,
buf.data<unsigned char>() + total))
return WebCryptoCipherStatus::FAILED;
total += tag_len;
}

// It's possible that we haven't used the full allocated space. Size down.
buf.Resize(total);
*out = std::move(buf);
*out = std::move(buf).release(total);

return WebCryptoCipherStatus::OK;
}
Expand Down Expand Up @@ -295,38 +292,34 @@ WebCryptoCipherStatus AES_CTR_Cipher(
return WebCryptoCipherStatus::FAILED;
}

// Output size is identical to the input size
char* data = MallocOpenSSL<char>(in.size());
ByteSource buf = ByteSource::Allocated(data, in.size());
unsigned char* ptr = reinterpret_cast<unsigned char*>(data);
// Output size is identical to the input size.
ByteSource::Builder buf(in.size());

// Also just like in chromium's implementation, if we can process
// the input without wrapping the counter, we'll do it as a single
// call here. If we can't, we'll fallback to the a two-step approach
if (BN_cmp(remaining_until_reset.get(), num_output.get()) >= 0) {
auto status = AES_CTR_Cipher2(
key_data,
cipher_mode,
params,
in,
params.iv.data<unsigned char>(),
ptr);
if (status == WebCryptoCipherStatus::OK)
*out = std::move(buf);
auto status = AES_CTR_Cipher2(key_data,
cipher_mode,
params,
in,
params.iv.data<unsigned char>(),
buf.data<unsigned char>());
if (status == WebCryptoCipherStatus::OK) *out = std::move(buf).release();
return status;
}

BN_ULONG blocks_part1 = BN_get_word(remaining_until_reset.get());
BN_ULONG input_size_part1 = blocks_part1 * kAesBlockSize;

// Encrypt the first part...
auto status = AES_CTR_Cipher2(
key_data,
cipher_mode,
params,
ByteSource::Foreign(in.get(), input_size_part1),
params.iv.data<unsigned char>(),
ptr);
auto status =
AES_CTR_Cipher2(key_data,
cipher_mode,
params,
ByteSource::Foreign(in.data<char>(), input_size_part1),
params.iv.data<unsigned char>(),
buf.data<unsigned char>());

if (status != WebCryptoCipherStatus::OK)
return status;
Expand All @@ -335,18 +328,16 @@ WebCryptoCipherStatus AES_CTR_Cipher(
std::vector<unsigned char> new_counter_block = BlockWithZeroedCounter(params);

// Encrypt the second part...
status = AES_CTR_Cipher2(
key_data,
cipher_mode,
params,
ByteSource::Foreign(
in.get() + input_size_part1,
in.size() - input_size_part1),
new_counter_block.data(),
ptr + input_size_part1);

if (status == WebCryptoCipherStatus::OK)
*out = std::move(buf);
status =
AES_CTR_Cipher2(key_data,
cipher_mode,
params,
ByteSource::Foreign(in.data<char>() + input_size_part1,
in.size() - input_size_part1),
new_counter_block.data(),
buf.data<unsigned char>() + input_size_part1);

if (status == WebCryptoCipherStatus::OK) *out = std::move(buf).release();

return status;
}
Expand Down
3 changes: 1 addition & 2 deletions src/crypto/crypto_common.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,8 +525,7 @@ MaybeLocal<Value> GetSerialNumber(Environment* env, X509* cert) {
if (bn) {
char* data = BN_bn2hex(bn.get());
ByteSource buf = ByteSource::Allocated(data, strlen(data));
if (buf)
return OneByteString(env->isolate(), buf.get());
if (buf) return OneByteString(env->isolate(), buf.data<unsigned char>());
}
}

Expand Down
13 changes: 4 additions & 9 deletions src/crypto/crypto_dh.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,18 +606,13 @@ ByteSource StatelessDiffieHellmanThreadsafe(
EVP_PKEY_derive(ctx.get(), nullptr, &out_size) <= 0)
return ByteSource();

char* buf = MallocOpenSSL<char>(out_size);
ByteSource out = ByteSource::Allocated(buf, out_size);

if (EVP_PKEY_derive(
ctx.get(),
reinterpret_cast<unsigned char*>(buf),
&out_size) <= 0) {
ByteSource::Builder out(out_size);
if (EVP_PKEY_derive(ctx.get(), out.data<unsigned char>(), &out_size) <= 0) {
return ByteSource();
}

ZeroPadDiffieHellmanSecret(out_size, buf, out.size());
return out;
ZeroPadDiffieHellmanSecret(out_size, out.data<char>(), out.size());
return std::move(out).release();
}
} // namespace

Expand Down
82 changes: 31 additions & 51 deletions src/crypto/crypto_ec.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,12 +486,9 @@ Maybe<bool> ECDHBitsTraits::AdditionalConfig(
return Just(true);
}

bool ECDHBitsTraits::DeriveBits(
Environment* env,
const ECDHBitsConfig& params,
ByteSource* out) {

char* data = nullptr;
bool ECDHBitsTraits::DeriveBits(Environment* env,
const ECDHBitsConfig& params,
ByteSource* out) {
size_t len = 0;
ManagedEVPPKey m_privkey = params.private_->GetAsymmetricKey();
ManagedEVPPKey m_pubkey = params.public_->GetAsymmetricKey();
Expand All @@ -513,15 +510,14 @@ bool ECDHBitsTraits::DeriveBits(
return false;
}

data = MallocOpenSSL<char>(len);
ByteSource::Builder buf(len);

if (EVP_PKEY_derive(
ctx.get(),
reinterpret_cast<unsigned char*>(data),
&len) <= 0) {
if (EVP_PKEY_derive(ctx.get(), buf.data<unsigned char>(), &len) <= 0) {
return false;
}

*out = std::move(buf).release(len);

break;
}
default: {
Expand All @@ -543,22 +539,18 @@ bool ECDHBitsTraits::DeriveBits(
const EC_POINT* pub = EC_KEY_get0_public_key(public_key);
int field_size = EC_GROUP_get_degree(group);
len = (field_size + 7) / 8;
data = MallocOpenSSL<char>(len);
CHECK_NOT_NULL(data);
ByteSource::Builder buf(len);
CHECK_NOT_NULL(pub);
CHECK_NOT_NULL(private_key);
if (ECDH_compute_key(
data,
len,
pub,
private_key,
nullptr) <= 0) {
if (ECDH_compute_key(buf.data<char>(), len, pub, private_key, nullptr) <=
0) {
return false;
}

*out = std::move(buf).release();
}
}
ByteSource buf = ByteSource::Allocated(data, len);
*out = std::move(buf);

return true;
}

Expand Down Expand Up @@ -646,7 +638,6 @@ WebCryptoKeyExportStatus EC_Raw_Export(

const EC_KEY* ec_key = EVP_PKEY_get0_EC_KEY(m_pkey.get());

unsigned char* data;
size_t len = 0;

if (ec_key == nullptr) {
Expand All @@ -666,9 +657,10 @@ WebCryptoKeyExportStatus EC_Raw_Export(
// Get the size of the raw key data
if (fn(m_pkey.get(), nullptr, &len) == 0)
return WebCryptoKeyExportStatus::INVALID_KEY_TYPE;
data = MallocOpenSSL<unsigned char>(len);
if (fn(m_pkey.get(), data, &len) == 0)
ByteSource::Builder data(len);
if (fn(m_pkey.get(), data.data<unsigned char>(), &len) == 0)
return WebCryptoKeyExportStatus::INVALID_KEY_TYPE;
*out = std::move(data).release(len);
} else {
if (key_data->GetKeyType() != kKeyTypePublic)
return WebCryptoKeyExportStatus::INVALID_KEY_TYPE;
Expand All @@ -680,17 +672,16 @@ WebCryptoKeyExportStatus EC_Raw_Export(
len = EC_POINT_point2oct(group, point, form, nullptr, 0, nullptr);
if (len == 0)
return WebCryptoKeyExportStatus::FAILED;
data = MallocOpenSSL<unsigned char>(len);
size_t check_len =
EC_POINT_point2oct(group, point, form, data, len, nullptr);
ByteSource::Builder data(len);
size_t check_len = EC_POINT_point2oct(
group, point, form, data.data<unsigned char>(), len, nullptr);
if (check_len == 0)
return WebCryptoKeyExportStatus::FAILED;

CHECK_EQ(len, check_len);
*out = std::move(data).release();
}

*out = ByteSource::Allocated(reinterpret_cast<char*>(data), len);

return WebCryptoKeyExportStatus::OK;
}
} // namespace
Expand Down Expand Up @@ -853,38 +844,27 @@ Maybe<bool> ExportJWKEdKey(
if (!EVP_PKEY_get_raw_public_key(pkey.get(), nullptr, &len))
return Nothing<bool>();

unsigned char* data = MallocOpenSSL<unsigned char>(len);
ByteSource out = ByteSource::Allocated(reinterpret_cast<char*>(data), len);
ByteSource::Builder out(len);

if (key->GetKeyType() == kKeyTypePrivate) {
if (!EVP_PKEY_get_raw_private_key(pkey.get(), data, &len) ||
if (!EVP_PKEY_get_raw_private_key(
pkey.get(), out.data<unsigned char>(), &len) ||
!StringBytes::Encode(
env->isolate(),
reinterpret_cast<const char*>(data),
len,
BASE64URL,
&error).ToLocal(&encoded) ||
!target->Set(
env->context(),
env->jwk_d_string(),
encoded).IsJust()) {
env->isolate(), out.data<const char>(), len, BASE64URL, &error)
.ToLocal(&encoded) ||
!target->Set(env->context(), env->jwk_d_string(), encoded).IsJust()) {
if (!error.IsEmpty())
env->isolate()->ThrowException(error);
return Nothing<bool>();
}
}

if (!EVP_PKEY_get_raw_public_key(pkey.get(), data, &len) ||
if (!EVP_PKEY_get_raw_public_key(
pkey.get(), out.data<unsigned char>(), &len) ||
!StringBytes::Encode(
env->isolate(),
reinterpret_cast<const char*>(data),
len,
BASE64URL,
&error).ToLocal(&encoded) ||
!target->Set(
env->context(),
env->jwk_x_string(),
encoded).IsJust()) {
env->isolate(), out.data<const char>(), len, BASE64URL, &error)
.ToLocal(&encoded) ||
!target->Set(env->context(), env->jwk_x_string(), encoded).IsJust()) {
if (!error.IsEmpty())
env->isolate()->ThrowException(error);
return Nothing<bool>();
Expand Down
Loading