From 7f605215f212809f69624348ce495b41407a6735 Mon Sep 17 00:00:00 2001 From: Sergey Chernyshev Date: Thu, 25 Apr 2024 00:14:10 +0200 Subject: [PATCH] src: fix TLSWrap lifetime bug in ALPN callback Retrieve the TLSWrap from the SSL object, not SSL_CTX. A SSL_CTX object is the parent of zero or more SSL objects. TLSWrap is a wrapper around SSL, SecureContext around SSL_CTX. Node.js normally uses a SecureContext per TLSWrap but it is possible to use a SecureContext object more than once. It was therefore possible for an ALPN callback to use the wrong (possibly already freed) TLSWrap object. Having said that, while the bug is clear once you see it, I'm not able to trigger it (and hence no test, not for lack of trying.) None of the bug reporters were able to reliably reproduce it either so the stars probably need to align just right in order to hit it. Fixes: /~https://github.com/nodejs/node/issues/47207 PR-URL: /~https://github.com/nodejs/node/pull/49635 Reviewed-By: Luigi Pinca Reviewed-By: Minwoo Jung --- graal-nodejs/src/crypto/crypto_tls.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/graal-nodejs/src/crypto/crypto_tls.cc b/graal-nodejs/src/crypto/crypto_tls.cc index 9e06209fa50..b76cd1117ea 100644 --- a/graal-nodejs/src/crypto/crypto_tls.cc +++ b/graal-nodejs/src/crypto/crypto_tls.cc @@ -223,7 +223,7 @@ int SelectALPNCallback( const unsigned char* in, unsigned int inlen, void* arg) { - TLSWrap* w = static_cast(arg); + TLSWrap* w = static_cast(SSL_get_app_data(s)); if (w->alpn_callback_enabled_) { Environment* env = w->env(); HandleScope handle_scope(env->isolate()); @@ -1293,7 +1293,8 @@ void TLSWrap::EnableALPNCb(const FunctionCallbackInfo& args) { wrap->alpn_callback_enabled_ = true; SSL* ssl = wrap->ssl_.get(); - SSL_CTX_set_alpn_select_cb(SSL_get_SSL_CTX(ssl), SelectALPNCallback, wrap); + SSL_CTX* ssl_ctx = SSL_get_SSL_CTX(ssl); + SSL_CTX_set_alpn_select_cb(ssl_ctx, SelectALPNCallback, nullptr); } void TLSWrap::GetServername(const FunctionCallbackInfo& args) { @@ -1589,7 +1590,8 @@ void TLSWrap::SetALPNProtocols(const FunctionCallbackInfo& args) { } else { w->alpn_protos_ = std::vector( protos.data(), protos.data() + protos.length()); - SSL_CTX_set_alpn_select_cb(SSL_get_SSL_CTX(ssl), SelectALPNCallback, w); + SSL_CTX* ssl_ctx = SSL_get_SSL_CTX(ssl); + SSL_CTX_set_alpn_select_cb(ssl_ctx, SelectALPNCallback, nullptr); } }