Skip to content
This repository has been archived by the owner on Nov 22, 2024. It is now read-only.

Commit

Permalink
Don't leak CRYPTO objects in generateCertPKCS12
Browse files Browse the repository at this point in the history
Summary:
`bioFromFile` is still allocating a BIO object which is not free'd.

I'm trying to chase a memory leak in media creation in IG therefore I need a local build with malloc tracing enabled and this leak it's introducing noise in my leaks tool analysis.

(I tried to disable flipper for local builds but it's more difficult than actually fixing this leak)

Reviewed By: fabiomassimo

Differential Revision: D65686212

fbshipit-source-id: 5d97dc37815ce15f09aeef554c9049c47234233f
  • Loading branch information
Andrea Tullis authored and facebook-github-bot committed Nov 11, 2024
1 parent 2e30968 commit 7aebeb2
Showing 1 changed file with 25 additions and 12 deletions.
37 changes: 25 additions & 12 deletions xplat/Flipper/CertificateUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -289,11 +289,6 @@ bool generateCertPKCS12(
OpenSSL_add_all_algorithms();
ERR_load_crypto_strings();

// Load the certificate's private key
if ((cert_privkey = EVP_PKEY_new()) == NULL) {
return false;
}

BIO* privateKeyBio = bioFromFile(keyFilepath);
if (privateKeyBio == nullptr) {
generateCertPKCS12_free(
Expand All @@ -303,6 +298,7 @@ bool generateCertPKCS12(

if (!(cert_privkey =
PEM_read_bio_PrivateKey(privateKeyBio, NULL, NULL, NULL))) {
BIO_free(privateKeyBio);
generateCertPKCS12_free(
cacert, cert, cert_privkey, cacertstack, pkcs12bundle);
return false;
Expand All @@ -311,12 +307,15 @@ bool generateCertPKCS12(
// Load the certificate
BIO* certificateBio = bioFromFile(certFilepath);
if (certificateBio == nullptr) {
BIO_free(privateKeyBio);
generateCertPKCS12_free(
cacert, cert, cert_privkey, cacertstack, pkcs12bundle);
return false;
}

if (!(cert = PEM_read_bio_X509(certificateBio, NULL, NULL, NULL))) {
BIO_free(privateKeyBio);
BIO_free(certificateBio);
generateCertPKCS12_free(
cacert, cert, cert_privkey, cacertstack, pkcs12bundle);
return false;
Expand All @@ -325,33 +324,34 @@ bool generateCertPKCS12(
// Load the CA certificate who signed it
BIO* cacertBio = bioFromFile(cacertFilepath);
if (cacertBio == nullptr) {
BIO_free(privateKeyBio);
BIO_free(certificateBio);
generateCertPKCS12_free(
cacert, cert, cert_privkey, cacertstack, pkcs12bundle);
return false;
}

if (!(cacert = PEM_read_bio_X509(cacertBio, NULL, NULL, NULL))) {
BIO_free(cacertBio);
BIO_free(privateKeyBio);
BIO_free(certificateBio);
generateCertPKCS12_free(
cacert, cert, cert_privkey, cacertstack, pkcs12bundle);
return false;
}

// Load the CA certificate on the stack
if ((cacertstack = sk_X509_new_null()) == NULL) {
BIO_free(cacertBio);
BIO_free(privateKeyBio);
BIO_free(certificateBio);
generateCertPKCS12_free(
cacert, cert, cert_privkey, cacertstack, pkcs12bundle);
return false;
}

sk_X509_push(cacertstack, cacert);

// Create the PKCS12 structure and fill it with our data
if ((pkcs12bundle = PKCS12_new()) == NULL) {
generateCertPKCS12_free(
cacert, cert, cert_privkey, cacertstack, pkcs12bundle);
return false;
}

// Values of zero use the openssl default values
pkcs12bundle = PKCS12_create(
const_cast<char*>(pkcs12Password), // certbundle access password
Expand All @@ -367,6 +367,9 @@ bool generateCertPKCS12(
);

if (pkcs12bundle == nullptr) {
BIO_free(cacertBio);
BIO_free(privateKeyBio);
BIO_free(certificateBio);
generateCertPKCS12_free(
cacert, cert, cert_privkey, cacertstack, pkcs12bundle);
return false;
Expand All @@ -376,19 +379,29 @@ bool generateCertPKCS12(
BIO* pkcs12Bio = BIO_new(BIO_s_mem());
bytes = i2d_PKCS12_bio(pkcs12Bio, pkcs12bundle);
if (bytes <= 0) {
BIO_free(cacertBio);
BIO_free(privateKeyBio);
BIO_free(certificateBio);
BIO_free(pkcs12Bio);
generateCertPKCS12_free(
cacert, cert, cert_privkey, cacertstack, pkcs12bundle);
return false;
}

if (!bioToFile(pkcs12Filepath, pkcs12Bio)) {
BIO_free(cacertBio);
BIO_free(privateKeyBio);
BIO_free(certificateBio);
BIO_free(pkcs12Bio);
generateCertPKCS12_free(
cacert, cert, cert_privkey, cacertstack, pkcs12bundle);
return false;
}

// Done, free resources
BIO_free(cacertBio);
BIO_free(privateKeyBio);
BIO_free(certificateBio);
BIO_free(pkcs12Bio);
generateCertPKCS12_free(
cacert, cert, cert_privkey, cacertstack, pkcs12bundle);
Expand Down

0 comments on commit 7aebeb2

Please sign in to comment.