From 2aeaab65d53b850d4ef53c661e2d9abbe0e26a4a 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 0662baa05ff..e04c74d13ee 100644 --- a/src/mumble/Cert.cpp +++ b/src/mumble/Cert.cpp @@ -434,8 +434,7 @@ Settings::KeyPair CertWizard::importCert(QByteArray data, const QString &pw) { Settings::KeyPair kp; int ret = 0; - mem = BIO_new_mem_buf(data.data(), data.size()); - Q_UNUSED(BIO_set_close(mem, BIO_NOCLOSE)); + mem = BIO_new_mem_buf(data.data(), 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 12b1f67d63a..ea01ad98605 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(), qbaKey.size()); - Q_UNUSED(BIO_set_close(mem, BIO_NOCLOSE)); + mem = BIO_new_mem_buf(qbaKey.data(), qbaKey.size()); pkey = d2i_PrivateKey_bio(mem, nullptr); BIO_free(mem); - mem = BIO_new_mem_buf(qbaCert.data(), qbaCert.size()); - Q_UNUSED(BIO_set_close(mem, BIO_NOCLOSE)); + mem = BIO_new_mem_buf(qbaCert.data(), qbaCert.size()); x509 = d2i_X509_bio(mem, nullptr); BIO_free(mem); mem = nullptr;