Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TLSSocket.prependOnceListener('session') results in an abort #38135

Closed
zyscoder opened this issue Apr 7, 2021 · 3 comments
Closed

TLSSocket.prependOnceListener('session') results in an abort #38135

zyscoder opened this issue Apr 7, 2021 · 3 comments
Labels
confirmed-bug Issues with confirmed bugs. tls Issues and PRs related to the tls subsystem.

Comments

@zyscoder
Copy link

zyscoder commented Apr 7, 2021

What steps will reproduce the bug?

Setup a node instance,

» node

and run the following javascript code.

tlsSocket = tls.connect('r','\u{85}');
tlsSocket.prependOnceListener('session',(session)=>{});

Then an abort occurs.
This PoC is very similar to #38133, but the core dump traces and the behaviors of nodejs are different. I think it's worth analyzing for both of them.

How often does it reproduce? Is there a required condition?

This problem can always be triggered following the steps above.

What is the expected behavior?

If any error occurs, an exception or other similar error-reporting stuff should be thrown. There is no reason to abort the whole node process.

What do you see instead?

» node
Welcome to Node.js v14.15.1.
Type ".help" for more information.
> tlsSocket = tls.connect('r','\u{85}');
<ref *1> TLSSocket {
...
}
> Uncaught:
<ref *1> Error: connect ENOENT r
...
}
> tlsSocket.prependOnceListener('session',(session)=>{});
node[225836]: ../src/tls_wrap.cc:938:static void node::TLSWrap::EnableSessionCallbacks(const v8::FunctionCallbackInfo<v8::Value>&): Assertion `(wrap->ssl_) != nullptr' failed.
 1: 0xa03530 node::Abort() [node]
 2: 0xa035ae  [node]
 3: 0xb44f2a  [node]
 4: 0xbe369b  [node]
 5: 0xbe4c46  [node]
 6: 0xbe52c6 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [node]
 7: 0x13ff259  [node]
[1]    225836 abort (core dumped)  node

Additional information

@Ayase-252 Ayase-252 added the tls Issues and PRs related to the tls subsystem. label Apr 7, 2021
@zyscoder
Copy link
Author

zyscoder commented Apr 7, 2021

BTW, OCSPResponse and secureConnect would not crash the node.js instance.

@aduh95 aduh95 added confirmed-bug Issues with confirmed bugs. repl Issues and PRs related to the REPL subsystem. labels Apr 7, 2021
@aduh95
Copy link
Contributor

aduh95 commented Apr 7, 2021

Can reproduce on master and v14.x, but only in REPL: cannot reproduce with eval (node -e, node -p, echo | node) or when inside a file.

@addaleax addaleax removed the repl Issues and PRs related to the REPL subsystem. label Apr 7, 2021
@addaleax
Copy link
Member

addaleax commented Apr 7, 2021

Not related to the REPL.

const tls = require('tls');
tlsSocket = tls.connect('r','\u{85}');
tlsSocket.on('error', () => {})
setTimeout(() => {
tlsSocket.prependOnceListener('session',(session)=>{});
}, 1000)

Linkgoron added a commit to Linkgoron/node that referenced this issue Apr 9, 2021
Fix an issue where adding a session or keylog listener
on a tlsSocket caused an abort.

fixes: nodejs#38133
fixes: nodejs#38135
Linkgoron added a commit to Linkgoron/node that referenced this issue Apr 9, 2021
Fix an issue where adding a session or keylog listener on a tlsSocket
after it was destroyed caused a segfault.

fixes: nodejs#38133
fixes: nodejs#38135
Linkgoron added a commit to Linkgoron/node that referenced this issue Apr 9, 2021
Fix an issue where adding a session or keylog listener on a tlsSocket
after it was destroyed caused a segfault.

fixes: nodejs#38133
fixes: nodejs#38135
Linkgoron added a commit to Linkgoron/node that referenced this issue Apr 9, 2021
Fix an issue where adding a session or keylog listener on a tlsSocket
after it was destroyed caused a segfault.

fixes: nodejs#38133
fixes: nodejs#38135
Linkgoron added a commit to Linkgoron/node that referenced this issue Apr 9, 2021
Fix an issue where adding a session or keylog listener on a tlsSocket
after it was destroyed caused a segfault.

fixes: nodejs#38133
fixes: nodejs#38135
BethGriggs pushed a commit that referenced this issue Apr 15, 2021
Fix an issue where adding a session or keylog listener on a tlsSocket
after it was destroyed caused a segfault.

fixes: #38133
fixes: #38135

PR-URL: #38180
Fixes: #38133
Fixes: #38135
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this issue May 30, 2021
Fix an issue where adding a session or keylog listener on a tlsSocket
after it was destroyed caused a segfault.

fixes: #38133
fixes: #38135

PR-URL: #38180
Fixes: #38133
Fixes: #38135
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this issue Jun 5, 2021
Fix an issue where adding a session or keylog listener on a tlsSocket
after it was destroyed caused a segfault.

fixes: #38133
fixes: #38135

PR-URL: #38180
Fixes: #38133
Fixes: #38135
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this issue Jun 11, 2021
Fix an issue where adding a session or keylog listener on a tlsSocket
after it was destroyed caused a segfault.

fixes: #38133
fixes: #38135

PR-URL: #38180
Fixes: #38133
Fixes: #38135
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants