Skip to content

Commit fe2694a

Browse files
joyeecheungtargos
authored andcommitted
crypto: fix X509* leak in --use-system-ca
The X509 structures are never freed. Use ncrypto::X509Pointer to manage it automatically and move the X509* to PEM conversion into a helper to be reused by integration in other systems. PR-URL: #56832 Reviewed-By: James M Snell <[email protected]>
1 parent 3d957d1 commit fe2694a

File tree

1 file changed

+33
-23
lines changed

1 file changed

+33
-23
lines changed

src/crypto/crypto_context.cc

+33-23
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ using ncrypto::MarkPopErrorOnReturn;
3535
using ncrypto::SSLPointer;
3636
using ncrypto::StackOfX509;
3737
using ncrypto::X509Pointer;
38+
using ncrypto::X509View;
3839
using v8::Array;
3940
using v8::ArrayBufferView;
4041
using v8::Boolean;
@@ -255,6 +256,35 @@ bool isSelfIssued(X509* cert) {
255256
return X509_NAME_cmp(subject, issuer) == 0;
256257
}
257258

259+
// TODO(joyeecheung): it is a bit excessive to do this X509 -> PEM -> X509
260+
// dance when we could've just pass everything around in binary. Change the
261+
// root_certs to be embedded as DER so that we can save the serialization
262+
// and deserialization.
263+
void X509VectorToPEMVector(const std::vector<X509Pointer>& src,
264+
std::vector<std::string>* dest) {
265+
for (size_t i = 0; i < src.size(); i++) {
266+
X509View x509_view(src[i].get());
267+
268+
auto pem_bio = x509_view.toPEM();
269+
if (!pem_bio) {
270+
fprintf(stderr,
271+
"Warning: converting system certificate to PEM format failed\n");
272+
continue;
273+
}
274+
275+
char* pem_data = nullptr;
276+
auto pem_size = BIO_get_mem_data(pem_bio.get(), &pem_data);
277+
if (pem_size <= 0 || !pem_data) {
278+
fprintf(
279+
stderr,
280+
"Warning: cannot read PEM-encoded data from system certificate\n");
281+
continue;
282+
}
283+
284+
dest->emplace_back(pem_data, pem_size);
285+
}
286+
}
287+
258288
#ifdef __APPLE__
259289
// This code is loosely based on
260290
// https://github.com/chromium/chromium/blob/54bd8e3/net/cert/internal/trust_store_mac.cc
@@ -467,7 +497,7 @@ void ReadMacOSKeychainCertificates(
467497

468498
CFIndex count = CFArrayGetCount(curr_anchors);
469499

470-
std::vector<X509*> system_root_certificates_X509;
500+
std::vector<X509Pointer> system_root_certificates_X509;
471501
for (int i = 0; i < count; ++i) {
472502
SecCertificateRef cert_ref = reinterpret_cast<SecCertificateRef>(
473503
const_cast<void*>(CFArrayGetValueAtIndex(curr_anchors, i)));
@@ -489,28 +519,8 @@ void ReadMacOSKeychainCertificates(
489519
}
490520
CFRelease(curr_anchors);
491521

492-
for (size_t i = 0; i < system_root_certificates_X509.size(); i++) {
493-
ncrypto::X509View x509_view(system_root_certificates_X509[i]);
494-
495-
auto pem_bio = x509_view.toPEM();
496-
if (!pem_bio) {
497-
fprintf(stderr,
498-
"Warning: converting system certificate to PEM format failed\n");
499-
continue;
500-
}
501-
502-
char* pem_data = nullptr;
503-
auto pem_size = BIO_get_mem_data(pem_bio.get(), &pem_data);
504-
if (pem_size <= 0 || !pem_data) {
505-
fprintf(
506-
stderr,
507-
"Warning: cannot read PEM-encoded data from system certificate\n");
508-
continue;
509-
}
510-
std::string certificate_string_pem(pem_data, pem_size);
511-
512-
system_root_certificates->emplace_back(certificate_string_pem);
513-
}
522+
X509VectorToPEMVector(system_root_certificates_X509,
523+
system_root_certificates);
514524
}
515525
#endif // __APPLE__
516526

0 commit comments

Comments
 (0)