From d316ae8e830b8b86f81307d35583f20773da6a55 Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Fri, 26 Feb 2021 12:11:03 +0100 Subject: [PATCH] QSsl: fix UB pointer use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Weird macro had inverted notion of type safety: instead of casting parameters and return values (they are pointers) to work with generic OPENSSL_sk_xxx functions, it was ... casting a function pointer to an invalid type to get ... nothing, but UB. Home-brewed (un)'safestack'!!! Change-Id: Ib91a7ba4cd472f370836797e422456f91a4385b0 Reviewed-by: Mårten Nordheim Reviewed-by: Edward Welbourne (cherry picked from commit 9fc2f1f076b953ff0979fb73ed6e70ac9bde278c) Reviewed-by: Qt Cherry-pick Bot --- src/network/ssl/qsslsocket_openssl_symbols_p.h | 10 +++++----- src/network/ssl/qx509_openssl.cpp | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/network/ssl/qsslsocket_openssl_symbols_p.h b/src/network/ssl/qsslsocket_openssl_symbols_p.h index 9f54efddaa..443faf068f 100644 --- a/src/network/ssl/qsslsocket_openssl_symbols_p.h +++ b/src/network/ssl/qsslsocket_openssl_symbols_p.h @@ -269,8 +269,8 @@ int q_DH_bits(DH *dh); # define q_SSL_load_error_strings() q_OPENSSL_init_ssl(OPENSSL_INIT_LOAD_SSL_STRINGS \ | OPENSSL_INIT_LOAD_CRYPTO_STRINGS, NULL) -#define q_SKM_sk_num(type, st) ((int (*)(const STACK_OF(type) *))q_OPENSSL_sk_num)(st) -#define q_SKM_sk_value(type, st,i) ((type * (*)(const STACK_OF(type) *, int))q_OPENSSL_sk_value)(st, i) +#define q_SKM_sk_num(st) q_OPENSSL_sk_num((OPENSSL_STACK *)st) +#define q_SKM_sk_value(type, st,i) (type *)q_OPENSSL_sk_value((OPENSSL_STACK *)st, i) #define q_OPENSSL_add_all_algorithms_conf() q_OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_CIPHERS \ | OPENSSL_INIT_ADD_ALL_DIGESTS \ @@ -612,14 +612,14 @@ void q_PKCS12_free(PKCS12 *pkcs12); #define q_BIO_get_mem_data(b, pp) (int)q_BIO_ctrl(b,BIO_CTRL_INFO,0,(char *)pp) #define q_BIO_pending(b) (int)q_BIO_ctrl(b,BIO_CTRL_PENDING,0,NULL) #define q_SSL_CTX_set_mode(ctx,op) q_SSL_CTX_ctrl((ctx),SSL_CTRL_MODE,(op),NULL) -#define q_sk_GENERAL_NAME_num(st) q_SKM_sk_num(GENERAL_NAME, (st)) +#define q_sk_GENERAL_NAME_num(st) q_SKM_sk_num((st)) #define q_sk_GENERAL_NAME_value(st, i) q_SKM_sk_value(GENERAL_NAME, (st), (i)) void q_GENERAL_NAME_free(GENERAL_NAME *a); -#define q_sk_X509_num(st) q_SKM_sk_num(X509, (st)) +#define q_sk_X509_num(st) q_SKM_sk_num((st)) #define q_sk_X509_value(st, i) q_SKM_sk_value(X509, (st), (i)) -#define q_sk_SSL_CIPHER_num(st) q_SKM_sk_num(SSL_CIPHER, (st)) +#define q_sk_SSL_CIPHER_num(st) q_SKM_sk_num((st)) #define q_sk_SSL_CIPHER_value(st, i) q_SKM_sk_value(SSL_CIPHER, (st), (i)) #define q_SSL_CTX_add_extra_chain_cert(ctx,x509) \ q_SSL_CTX_ctrl(ctx,SSL_CTRL_EXTRA_CHAIN_CERT,0,(char *)x509) diff --git a/src/network/ssl/qx509_openssl.cpp b/src/network/ssl/qx509_openssl.cpp index 376b24ab37..ad9c55463b 100644 --- a/src/network/ssl/qx509_openssl.cpp +++ b/src/network/ssl/qx509_openssl.cpp @@ -194,7 +194,7 @@ QVariant x509UnknownExtensionToValue(X509_EXTENSION *ext) QVariantList list; bool isMap = false; - for (int j = 0; j < q_SKM_sk_num(CONF_VALUE, val); j++) { + for (int j = 0; j < q_SKM_sk_num(val); j++) { CONF_VALUE *nval = q_SKM_sk_value(CONF_VALUE, val, j); if (nval->name && nval->value) { isMap = true; @@ -263,7 +263,7 @@ QVariant x509ExtensionToValue(X509_EXTENSION *ext) if (!info) return {}; QVariantMap result; - for (int i=0; i < q_SKM_sk_num(ACCESS_DESCRIPTION, info); i++) { + for (int i=0; i < q_SKM_sk_num(info); i++) { ACCESS_DESCRIPTION *ad = q_SKM_sk_value(ACCESS_DESCRIPTION, info, i); GENERAL_NAME *name = ad->location;