QSslCertificate(OpenSSL plugin): fix memory leaks in extension 'parser'

They went unnoticed previously because of lazy evaluation, which is
not the case anymore.

Fixes: QTBUG-96155
Pick-to: 6.2 5.15
Change-Id: I46026a24b354c1db7c10d84fceae06c4ab7cc0fc
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
bb10
Timur Pocheptsov 2021-09-02 11:01:16 +02:00
parent ee5077daf5
commit c55f61578c
3 changed files with 63 additions and 12 deletions

View File

@ -181,6 +181,8 @@ DEFINEFUNC(const SSL_METHOD *, TLS_server_method, DUMMYARG, DUMMYARG, return nul
DEFINEFUNC(void, X509_up_ref, X509 *a, a, return, DUMMYARG)
DEFINEFUNC(ASN1_TIME *, X509_getm_notBefore, X509 *a, a, return nullptr, return)
DEFINEFUNC(ASN1_TIME *, X509_getm_notAfter, X509 *a, a, return nullptr, return)
DEFINEFUNC2(void, ASN1_item_free, ASN1_VALUE *val, val, const ASN1_ITEM *it, it, return, return)
DEFINEFUNC(void, X509V3_conf_free, CONF_VALUE *val, val, return, return)
DEFINEFUNC(long, X509_get_version, X509 *a, a, return -1, return)
DEFINEFUNC(EVP_PKEY *, X509_get_pubkey, X509 *a, a, return nullptr, return)
DEFINEFUNC2(void, X509_STORE_set_verify_cb, X509_STORE *a, a, X509_STORE_CTX_verify_cb verify_cb, verify_cb, return, DUMMYARG)
@ -236,6 +238,7 @@ DEFINEFUNC6(int, OCSP_basic_sign, OCSP_BASICRESP *br, br, X509 *signer, signer,
const EVP_MD *dg, dg, STACK_OF(X509) *cs, cs, unsigned long flags, flags, return 0, return)
#endif // ocsp
DEFINEFUNC(void, AUTHORITY_INFO_ACCESS_free, AUTHORITY_INFO_ACCESS *p, p, return, return)
DEFINEFUNC2(void, BIO_set_data, BIO *a, a, void *ptr, ptr, return, DUMMYARG)
DEFINEFUNC(void *, BIO_get_data, BIO *a, a, return nullptr, return)
DEFINEFUNC2(void, BIO_set_init, BIO *a, a, int init, init, return, DUMMYARG)
@ -873,6 +876,7 @@ bool q_resolveOpenSslSymbols()
RESOLVEFUNC(OPENSSL_init_crypto)
RESOLVEFUNC(ASN1_STRING_get0_data)
RESOLVEFUNC(EVP_CIPHER_CTX_reset)
RESOLVEFUNC(AUTHORITY_INFO_ACCESS_free)
RESOLVEFUNC(EVP_PKEY_up_ref)
RESOLVEFUNC(EVP_PKEY_CTX_new)
RESOLVEFUNC(EVP_PKEY_param_check)
@ -910,6 +914,8 @@ bool q_resolveOpenSslSymbols()
RESOLVEFUNC(X509_STORE_CTX_get0_chain)
RESOLVEFUNC(X509_getm_notBefore)
RESOLVEFUNC(X509_getm_notAfter)
RESOLVEFUNC(ASN1_item_free)
RESOLVEFUNC(X509V3_conf_free)
RESOLVEFUNC(X509_get_version)
RESOLVEFUNC(X509_get_pubkey)
RESOLVEFUNC(X509_STORE_set_verify_cb)

View File

@ -232,6 +232,7 @@ const unsigned char * q_ASN1_STRING_get0_data(const ASN1_STRING *x);
BIO *q_BIO_new(const BIO_METHOD *a);
const BIO_METHOD *q_BIO_s_mem();
void q_AUTHORITY_INFO_ACCESS_free(AUTHORITY_INFO_ACCESS *a);
int q_EVP_CIPHER_CTX_reset(EVP_CIPHER_CTX *c);
int q_EVP_PKEY_up_ref(EVP_PKEY *a);
EVP_PKEY_CTX *q_EVP_PKEY_CTX_new(EVP_PKEY *pkey, ENGINE *e);
@ -254,6 +255,8 @@ const SSL_METHOD *q_TLS_client_method();
const SSL_METHOD *q_TLS_server_method();
ASN1_TIME *q_X509_getm_notBefore(X509 *a);
ASN1_TIME *q_X509_getm_notAfter(X509 *a);
void q_ASN1_item_free(ASN1_VALUE *val, const ASN1_ITEM *it);
void q_X509V3_conf_free(CONF_VALUE *val);
void q_X509_up_ref(X509 *a);
long q_X509_get_version(X509 *a);
@ -277,7 +280,6 @@ void q_DH_get0_pqg(const DH *dh, const BIGNUM **p, const BIGNUM **q, const BIGNU
| OPENSSL_INIT_ADD_ALL_DIGESTS, NULL)
int q_OPENSSL_init_crypto(uint64_t opts, const OPENSSL_INIT_SETTINGS *settings);
void q_CRYPTO_free(void *str, const char *file, int line);
long q_OpenSSL_version_num();
const char *q_OpenSSL_version(int type);
@ -698,6 +700,8 @@ int q_OCSP_id_cmp(OCSP_CERTID *a, OCSP_CERTID *b);
void *q_CRYPTO_malloc(size_t num, const char *file, int line);
#define q_OPENSSL_malloc(num) q_CRYPTO_malloc(num, "", 0)
void q_CRYPTO_free(void *str, const char *file, int line);
# define q_OPENSSL_free(addr) q_CRYPTO_free(addr, "", 0)
void q_SSL_set_info_callback(SSL *ssl, void (*cb) (const SSL *ssl, int type, int val));
const char *q_SSL_alert_type_string(int value);

View File

@ -197,10 +197,27 @@ QVariant x509UnknownExtensionToValue(X509_EXTENSION *ext)
}
void *ext_internal = q_X509V3_EXT_d2i(ext);
if (!ext_internal)
return {};
const auto extCleaner = qScopeGuard([meth, ext_internal]{
Q_ASSERT(ext_internal && meth);
if (meth->it)
q_ASN1_item_free(static_cast<ASN1_VALUE *>(ext_internal), ASN1_ITEM_ptr(meth->it));
else if (meth->ext_free)
meth->ext_free(ext_internal);
else
qCWarning(lcTlsBackend, "No method to free an unknown extension, a potential memory leak?");
});
// If this extension can be converted
if (meth->i2v && ext_internal) {
if (meth->i2v) {
STACK_OF(CONF_VALUE) *val = meth->i2v(meth, ext_internal, nullptr);
const auto stackCleaner = qScopeGuard([val]{
if (val)
q_OPENSSL_sk_pop_free((OPENSSL_STACK *)val, (void(*)(void*))q_X509V3_conf_free);
});
QVariantMap map;
QVariantList list;
@ -222,10 +239,12 @@ QVariant x509UnknownExtensionToValue(X509_EXTENSION *ext)
return map;
else
return list;
} else if (meth->i2s && ext_internal) {
QVariant result(QString::fromUtf8(meth->i2s(meth, ext_internal)));
} else if (meth->i2s) {
const char *hexString = meth->i2s(meth, ext_internal);
QVariant result(hexString ? QString::fromUtf8(hexString) : QString{});
q_OPENSSL_free((void *)hexString);
return result;
} else if (meth->i2r && ext_internal) {
} else if (meth->i2r) {
QByteArray result;
BIO *bio = q_BIO_new(q_BIO_s_mem());
@ -254,6 +273,31 @@ QVariant x509ExtensionToValue(X509_EXTENSION *ext)
{
ASN1_OBJECT *obj = q_X509_EXTENSION_get_object(ext);
int nid = q_OBJ_obj2nid(obj);
// We cast away the const-ness here because some versions of openssl
// don't use const for the parameters in the functions pointers stored
// in the object.
X509V3_EXT_METHOD *meth = const_cast<X509V3_EXT_METHOD *>(q_X509V3_EXT_get(ext));
void *ext_internal = nullptr; // The value, returned by X509V3_EXT_d2i.
const auto extCleaner = qScopeGuard([meth, &ext_internal]() {
if (!meth || !ext_internal)
return;
if (meth->it)
q_ASN1_item_free(static_cast<ASN1_VALUE *>(ext_internal), ASN1_ITEM_ptr(meth->it));
else if (meth->ext_free)
meth->ext_free(ext_internal);
else
qWarning(lcTlsBackend, "Cannot free an extension, a potential memory leak?");
});
const char * hexString = nullptr; // The value returned by meth->i2s.
const auto hexStringCleaner = qScopeGuard([&hexString](){
if (hexString)
q_OPENSSL_free((void*)hexString);
});
switch (nid) {
case NID_basic_constraints:
{
@ -295,21 +339,18 @@ QVariant x509ExtensionToValue(X509_EXTENSION *ext)
}
}
q_OPENSSL_sk_pop_free((OPENSSL_STACK*)info, reinterpret_cast<void(*)(void *)>(q_OPENSSL_sk_free));
q_AUTHORITY_INFO_ACCESS_free(info);
return result;
}
break;
case NID_subject_key_identifier:
{
void *ext_internal = q_X509V3_EXT_d2i(ext);
ext_internal = q_X509V3_EXT_d2i(ext);
if (!ext_internal)
return {};
// we cast away the const-ness here because some versions of openssl
// don't use const for the parameters in the functions pointers stored
// in the object.
X509V3_EXT_METHOD *meth = const_cast<X509V3_EXT_METHOD *>(q_X509V3_EXT_get(ext));
return QVariant(QString::fromUtf8(meth->i2s(meth, ext_internal)));
hexString = meth->i2s(meth, ext_internal);
return QVariant(QString::fromUtf8(hexString));
}
break;
case NID_authority_key_identifier: