Skip to content

Commit

Permalink
FIX(client): Fix memory leaks due to BIO_NOCLOSE flag
Browse files Browse the repository at this point in the history
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
  • Loading branch information
botanegg authored and Krzmbrzl committed Jan 12, 2025
1 parent ba3a32d commit 1fcb27f
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 6 deletions.
3 changes: 1 addition & 2 deletions src/mumble/Cert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 2 additions & 4 deletions src/murmur/Cert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 1fcb27f

Please sign in to comment.