Skip to content

Commit d0868ff

Browse files
bnoordhuisMylesBorins
authored andcommitted
tls: fix segfault on destroy after partial read
OnRead() calls into JS land which can result in the SSL context object being destroyed on return. Check that `ssl_ != nullptr` afterwards. Fixes: #11885 PR-URL: #11898 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent f53c48e commit d0868ff

2 files changed

Lines changed: 42 additions & 0 deletions

File tree

‎src/tls_wrap.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,12 @@ void TLSWrap::ClearOut() {
426426
memcpy(buf.base, current, avail);
427427
OnRead(avail, &buf);
428428

429+
// Caveat emptor: OnRead() calls into JS land which can result in
430+
// the SSL context object being destroyed. We have to carefully
431+
// check that ssl_ != nullptr afterwards.
432+
if (ssl_ == nullptr)
433+
return;
434+
429435
read -= avail;
430436
current += avail;
431437
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
'use strict';
2+
3+
constcommon=require('../common');
4+
5+
if(!common.hasCrypto){
6+
common.skip('missing crypto');
7+
return;
8+
}
9+
10+
constfs=require('fs');
11+
constnet=require('net');
12+
consttls=require('tls');
13+
14+
constkey=fs.readFileSync(common.fixturesDir+'/keys/agent1-key.pem');
15+
constcert=fs.readFileSync(common.fixturesDir+'/keys/agent1-cert.pem');
16+
constsecureContext=tls.createSecureContext({ key, cert });
17+
18+
constserver=net.createServer(common.mustCall((conn)=>{
19+
constoptions={isServer: true, secureContext, server };
20+
constsocket=newtls.TLSSocket(conn,options);
21+
socket.once('data',common.mustCall(()=>{
22+
socket._destroySSL();// Should not crash.
23+
server.close();
24+
}));
25+
}));
26+
27+
server.listen(0,function(){
28+
constoptions={
29+
port: this.address().port,
30+
rejectUnauthorized: false,
31+
};
32+
tls.connect(options,function(){
33+
this.write('*'.repeat(1<<20));// Write more data than fits in a frame.
34+
this.on('error',this.destroy);// Server closes connection on us.
35+
});
36+
});

0 commit comments

Comments
 (0)