From 84380965a504a2a25450378de638df891c75d569 Mon Sep 17 00:00:00 2001 From: Valentine Krasnobaeva Date: Tue, 28 May 2024 10:49:51 +0200 Subject: [PATCH] BUG/MINOR: ssl/ocsp: init callback func ptr as NULL In ssl_sock_load_ocsp() it is better to initialize local scope variable 'callback' function pointer as NULL, while we are declaring it. According to SSL_CTX_get_tlsext_status_cb() API, then we will provide a pointer to this 'on stack' variable in order to check, if the callback was already set before: OpenSSL 1.x.x and 3.x.x: long SSL_CTX_get_tlsext_status_cb(SSL_CTX *ctx, int (**callback)(SSL *, void *)); long SSL_CTX_set_tlsext_status_cb(SSL_CTX *ctx, int (*callback)(SSL *, void *)); WolfSSL 5.7.0: typedef int(*tlsextStatusCb)(WOLFSSL* ssl, void*); WOLFSSL_API int wolfSSL_CTX_get_tlsext_status_cb(WOLFSSL_CTX* ctx, tlsextStatusCb* cb); WOLFSSL_API int wolfSSL_CTX_set_tlsext_status_cb(WOLFSSL_CTX* ctx, tlsextStatusCb cb); When this func ptr variable stays uninitialized, haproxy comipled with ASAN crushes in ssl_sock_load_ocsp(): ./haproxy -d -f haproxy.cfg ... AddressSanitizer:DEADLYSIGNAL ================================================================= ==114919==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000008 (pc 0x5eab8951bb32 bp 0x7ffcdd6d8410 sp 0x7ffcdd6d82e0 T0) ==114919==The signal is caused by a READ memory access. ==114919==Hint: address points to the zero page. #0 0x5eab8951bb32 in ssl_sock_load_ocsp /home/vk/projects/haproxy/src/ssl_sock.c:1248:22 #1 0x5eab89510d65 in ssl_sock_put_ckch_into_ctx /home/vk/projects/haproxy/src/ssl_sock.c:3389:6 ... This happens, because callback variable is allocated on the stack. As not being explicitly initialized, it may contain some garbage value at runtime, due to the linked crypto library update or recompilation. So, following ssl_sock_load_ocsp code, SSL_CTX_get_tlsext_status_cb() may fail, callback will still contain its initial garbage value, 'if (!callback) {...' test will put us on the wrong path to access some ocsp_cbk_arg properties via its pointer, which won't be set and like this we will finish with segmentation fault. Must be backported in all stable versions. All versions does not have the ifdef, the previous cleanup patch is useful starting from the 2.7 version. --- src/ssl_sock.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 81affd1d3..a907f595d 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -1116,11 +1116,12 @@ static int ssl_sock_load_ocsp(const char *path, SSL_CTX *ctx, struct ckch_store char *warn = NULL; unsigned char *p; #ifdef USE_OPENSSL_WOLFSSL - tlsextStatusCb callback; + /* typedef int(*tlsextStatusCb)(WOLFSSL* ssl, void*); */ + tlsextStatusCb callback = NULL; #elif (HA_OPENSSL_VERSION_NUMBER >= 0x10101000L) - int (*callback) (SSL *, void *); + int (*callback) (SSL *, void *) = NULL; #else - void (*callback) (void); + void (*callback) (void) = NULL; #endif struct buffer *ocsp_uri = get_trash_chunk(); char *err = NULL;