From 1fcb27f1fec64e43a1ad307f70d8ed2180c81f21 Mon Sep 17 00:00:00 2001 From: Konstantin Goncharik Date: Fri, 11 Oct 2024 19:50:41 +0700 Subject: [PATCH] FIX(client): Fix memory leaks due to BIO_NOCLOSE flag BIO_set_close with BIO_NOCLOSE argument leads to OpenSSL not (fully) deleting the allocated BIO struct under the assumption that the user code has taken ownership of it. However, in our case, this is not the case and therefore OpenSSL should do the deletion as usual. The flag was probably introduced under the assumption that the component that either is or isn't deleted by OpenSSL was the externally provided buffer that is wrapped into a BIO object via BIO_new_mem_buf. However, this is not the case. OpenSSL doesn't take ownership of the provided buffer and therefore also doesn't delete it. Closes #6603 --- src/mumble/Cert.cpp | 3 +-- src/murmur/Cert.cpp | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/mumble/Cert.cpp b/src/mumble/Cert.cpp index 9daabeaa879..6a538355c42 100644 --- a/src/mumble/Cert.cpp +++ b/src/mumble/Cert.cpp @@ -436,8 +436,7 @@ Settings::KeyPair CertWizard::importCert(QByteArray data, const QString &pw) { Settings::KeyPair kp; int ret = 0; - mem = BIO_new_mem_buf(data.data(), static_cast< int >(data.size())); - Q_UNUSED(BIO_set_close(mem, BIO_NOCLOSE)); + mem = BIO_new_mem_buf(data.data(), static_cast< int >(data.size())); pkcs = d2i_PKCS12_bio(mem, nullptr); if (pkcs) { ret = PKCS12_parse(pkcs, nullptr, &pkey, &x509, &certs); diff --git a/src/murmur/Cert.cpp b/src/murmur/Cert.cpp index 71ec363dfa3..9c38797c238 100644 --- a/src/murmur/Cert.cpp +++ b/src/murmur/Cert.cpp @@ -32,13 +32,11 @@ bool Server::isKeyForCert(const QSslKey &key, const QSslCertificate &cert) { EVP_PKEY *pkey = nullptr; BIO *mem = nullptr; - mem = BIO_new_mem_buf(qbaKey.data(), static_cast< int >(qbaKey.size())); - Q_UNUSED(BIO_set_close(mem, BIO_NOCLOSE)); + mem = BIO_new_mem_buf(qbaKey.data(), static_cast< int >(qbaKey.size())); pkey = d2i_PrivateKey_bio(mem, nullptr); BIO_free(mem); - mem = BIO_new_mem_buf(qbaCert.data(), static_cast< int >(qbaCert.size())); - Q_UNUSED(BIO_set_close(mem, BIO_NOCLOSE)); + mem = BIO_new_mem_buf(qbaCert.data(), static_cast< int >(qbaCert.size())); x509 = d2i_X509_bio(mem, nullptr); BIO_free(mem); mem = nullptr;