Skip to content

Commit ebda39a

Browse files
davidbendanbev
authored andcommitted
crypto: fix malloc mixing in X509ToObject
EC_KEY_key2buf returns an OPENSSL_malloc'd pointer so it shouldn't be passed into Buffer::New, which expect a libc malloc'd pointer. Instead, factor out the ECDH::GetPublicKey code which uses EC_POINT_point2oct. This preserves the existing behavior where encoding failures are silently ignored, but it is probably safe to CHECK fail them instead. PR-URL: #25717 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
1 parent 793c630 commit ebda39a

File tree

1 file changed

+30
-42
lines changed

1 file changed

+30
-42
lines changed

src/node_crypto.cc

Lines changed: 30 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1640,6 +1640,24 @@ static void AddFingerprintDigest(const unsigned char* md,
16401640
}
16411641
}
16421642

1643+
static MaybeLocal<Object> ECPointToBuffer(Environment* env,
1644+
const EC_GROUP* group,
1645+
const EC_POINT* point,
1646+
point_conversion_form_t form) {
1647+
size_t len = EC_POINT_point2oct(group, point, form, nullptr, 0, nullptr);
1648+
if (len == 0) {
1649+
env->ThrowError("Failed to get public key length");
1650+
return MaybeLocal<Object>();
1651+
}
1652+
MallocedBuffer<unsigned char> buf(len);
1653+
len = EC_POINT_point2oct(group, point, form, buf.data, buf.size, nullptr);
1654+
if (len == 0) {
1655+
env->ThrowError("Failed to get public key");
1656+
return MaybeLocal<Object>();
1657+
}
1658+
return Buffer::New(env, buf.release(), len);
1659+
}
1660+
16431661
static Local<Object> X509ToObject(Environment* env, X509* cert) {
16441662
EscapableHandleScope scope(env->isolate());
16451663
Local<Context> context = env->context();
@@ -1756,16 +1774,12 @@ static Local<Object> X509ToObject(Environment* env, X509* cert) {
17561774
}
17571775
}
17581776

1759-
unsigned char* pub = nullptr;
1760-
size_t publen = EC_KEY_key2buf(ec.get(), EC_KEY_get_conv_form(ec.get()),
1761-
&pub, nullptr);
1762-
if (publen > 0) {
1763-
Local<Object> buf = Buffer::New(env, pub, publen).ToLocalChecked();
1764-
// Ownership of pub pointer accepted by Buffer.
1765-
pub = nullptr;
1777+
const EC_POINT* pubkey = EC_KEY_get0_public_key(ec.get());
1778+
if (pubkey != nullptr) {
1779+
Local<Object> buf =
1780+
ECPointToBuffer(env, group, pubkey, EC_KEY_get_conv_form(ec.get()))
1781+
.ToLocalChecked();
17661782
info->Set(context, env->pubkey_string(), buf).FromJust();
1767-
} else {
1768-
CHECK_NULL(pub);
17691783
}
17701784

17711785
const int nid = EC_GROUP_get_curve_name(group);
@@ -5265,26 +5279,14 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo<Value>& args) {
52655279
if (pub == nullptr)
52665280
return env->ThrowError("Failed to get ECDH public key");
52675281

5268-
int size;
52695282
CHECK(args[0]->IsUint32());
52705283
uint32_t val = args[0].As<Uint32>()->Value();
52715284
point_conversion_form_t form = static_cast<point_conversion_form_t>(val);
52725285

5273-
size = EC_POINT_point2oct(ecdh->group_, pub, form, nullptr, 0, nullptr);
5274-
if (size == 0)
5275-
return env->ThrowError("Failed to get public key length");
5276-
5277-
unsigned char* out = node::Malloc<unsigned char>(size);
5278-
5279-
int r = EC_POINT_point2oct(ecdh->group_, pub, form, out, size, nullptr);
5280-
if (r != size) {
5281-
free(out);
5282-
return env->ThrowError("Failed to get public key");
5283-
}
5284-
5285-
Local<Object> buf =
5286-
Buffer::New(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
5287-
args.GetReturnValue().Set(buf);
5286+
MaybeLocal<Object> buf =
5287+
ECPointToBuffer(env, EC_KEY_get0_group(ecdh->key_.get()), pub, form);
5288+
if (buf.IsEmpty()) return;
5289+
args.GetReturnValue().Set(buf.ToLocalChecked());
52885290
}
52895291

52905292

@@ -6172,23 +6174,9 @@ void ConvertKey(const FunctionCallbackInfo<Value>& args) {
61726174
uint32_t val = args[2].As<Uint32>()->Value();
61736175
point_conversion_form_t form = static_cast<point_conversion_form_t>(val);
61746176

6175-
int size = EC_POINT_point2oct(
6176-
group.get(), pub.get(), form, nullptr, 0, nullptr);
6177-
6178-
if (size == 0)
6179-
return env->ThrowError("Failed to get public key length");
6180-
6181-
unsigned char* out = node::Malloc<unsigned char>(size);
6182-
6183-
int r = EC_POINT_point2oct(group.get(), pub.get(), form, out, size, nullptr);
6184-
if (r != size) {
6185-
free(out);
6186-
return env->ThrowError("Failed to get public key");
6187-
}
6188-
6189-
Local<Object> buf =
6190-
Buffer::New(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
6191-
args.GetReturnValue().Set(buf);
6177+
MaybeLocal<Object> buf = ECPointToBuffer(env, group.get(), pub.get(), form);
6178+
if (buf.IsEmpty()) return;
6179+
args.GetReturnValue().Set(buf.ToLocalChecked());
61926180
}
61936181

61946182

0 commit comments

Comments
 (0)