From fab2e99bff1bea243189f638c7a05bc5533c3130 Mon Sep 17 00:00:00 2001 From: Hayden Roche Date: Thu, 3 Feb 2022 09:07:14 -0800 Subject: [PATCH] Fix bug in TLSX_ALPN_ParseAndSet when using ALPN select callback. At the start of this function, it attempts to find an ALPN extension in the ssl object's extensions with `TLSX_Find`. If an ALPN select callback has been set (i.e. via `wolfSSL_CTX_set_alpn_select_cb`), that gets called next. If that callback finds a match, it removes all existing ALPN extensions found in the ssl object. It then uses the new protocol name like this: ``` if (TLSX_UseALPN(&ssl->extensions, (char*)out, outLen, 0, ssl->heap) == WOLFSSL_SUCCESS) { if (extension == NULL) { extension = TLSX_Find(ssl->extensions, TLSX_APPLICATION_LAYER_PROTOCOL); } } ``` The bug is exposed if `extension` is not NULL, i.e. it was found on that initial `TLSX_Find` call. `extension` is not NULL but it now points to garbage because all the old ALPN extensions were just removed. It won't have it's value assigned to the new extension that just got pushed via `TLSX_UseALPN` because of this NULL check. This results in a segfault later in the function. The solution is to remove the NULL check and always update `extension` after the `TLSX_UseALPN` call. This bug was discovered by a customer when using nginx + wolfSSL. I was able to reproduce locally with curl acting as the client --- src/tls.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/tls.c b/src/tls.c index f9dc7d345..8e0694874 100644 --- a/src/tls.c +++ b/src/tls.c @@ -1559,7 +1559,7 @@ static int TLSX_ALPN_ParseAndSet(WOLFSSL *ssl, const byte *input, word16 length, extension = TLSX_Find(ssl->extensions, TLSX_APPLICATION_LAYER_PROTOCOL); if (extension == NULL) extension = TLSX_Find(ssl->ctx->extensions, - TLSX_APPLICATION_LAYER_PROTOCOL); + TLSX_APPLICATION_LAYER_PROTOCOL); #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) if (ssl->alpnSelect != NULL && ssl->options.side == WOLFSSL_SERVER_END) { @@ -1571,13 +1571,11 @@ static int TLSX_ALPN_ParseAndSet(WOLFSSL *ssl, const byte *input, word16 length, WOLFSSL_MSG("ALPN protocol match"); /* clears out all current ALPN extensions set */ TLSX_Remove(&ssl->extensions, TLSX_APPLICATION_LAYER_PROTOCOL, ssl->heap); - + extension = NULL; if (TLSX_UseALPN(&ssl->extensions, (char*)out, outLen, 0, ssl->heap) == WOLFSSL_SUCCESS) { - if (extension == NULL) { - extension = TLSX_Find(ssl->extensions, - TLSX_APPLICATION_LAYER_PROTOCOL); - } + extension = TLSX_Find(ssl->extensions, + TLSX_APPLICATION_LAYER_PROTOCOL); } } }