Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 1643adf

Browse filesBrowse 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 <luigipinca@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
1 parent 66776d8 commit 1643adf
Copy full SHA for 1643adf

File tree

Expand file treeCollapse file tree

1 file changed

+5
-3
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

1 file changed

+5
-3
lines changed
Open diff view settings
Collapse file

‎src/crypto/crypto_tls.cc‎

Copy file name to clipboardExpand all lines: src/crypto/crypto_tls.cc
+5-3Lines changed: 5 additions & 3 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.