Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit fe2694a

Browse filesBrowse 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 <jasnell@gmail.com>
1 parent 3d957d1 commit fe2694a
Copy full SHA for fe2694a

File tree

Expand file treeCollapse file tree

1 file changed

+33
-23
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

1 file changed

+33
-23
lines changed
Open diff view settings
Collapse file

‎src/crypto/crypto_context.cc‎

Copy file name to clipboardExpand all lines: src/crypto/crypto_context.cc
+33-23Lines changed: 33 additions & 23 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.