Skip to content

Commit 1643adf

Browse files
authored
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: #47207 PR-URL: #49635 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
1 parent 66776d8 commit 1643adf

File tree

1 file changed

+5
-3
lines changed

1 file changed

+5
-3
lines changed

src/crypto/crypto_tls.cc

+5-3
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ int SelectALPNCallback(
223223
const unsigned char* in,
224224
unsigned int inlen,
225225
void* arg) {
226-
TLSWrap* w = static_cast<TLSWrap*>(arg);
226+
TLSWrap* w = static_cast<TLSWrap*>(SSL_get_app_data(s));
227227
if (w->alpn_callback_enabled_) {
228228
Environment* env = w->env();
229229
HandleScope handle_scope(env->isolate());
@@ -1293,7 +1293,8 @@ void TLSWrap::EnableALPNCb(const FunctionCallbackInfo<Value>& args) {
12931293
wrap->alpn_callback_enabled_ = true;
12941294

12951295
SSL* ssl = wrap->ssl_.get();
1296-
SSL_CTX_set_alpn_select_cb(SSL_get_SSL_CTX(ssl), SelectALPNCallback, wrap);
1296+
SSL_CTX* ssl_ctx = SSL_get_SSL_CTX(ssl);
1297+
SSL_CTX_set_alpn_select_cb(ssl_ctx, SelectALPNCallback, nullptr);
12971298
}
12981299

12991300
void TLSWrap::GetServername(const FunctionCallbackInfo<Value>& args) {
@@ -1589,7 +1590,8 @@ void TLSWrap::SetALPNProtocols(const FunctionCallbackInfo<Value>& args) {
15891590
} else {
15901591
w->alpn_protos_ = std::vector<unsigned char>(
15911592
protos.data(), protos.data() + protos.length());
1592-
SSL_CTX_set_alpn_select_cb(SSL_get_SSL_CTX(ssl), SelectALPNCallback, w);
1593+
SSL_CTX* ssl_ctx = SSL_get_SSL_CTX(ssl);
1594+
SSL_CTX_set_alpn_select_cb(ssl_ctx, SelectALPNCallback, nullptr);
15931595
}
15941596
}
15951597

0 commit comments

Comments
 (0)