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 54f5258

Browse filesBrowse 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 4426080 commit 54f5258
Copy full SHA for 54f5258

File tree

Expand file treeCollapse file tree

2 files changed

+42
-0
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+42
-0
lines changed
Open diff view settings
Collapse file

‎src/tls_wrap.cc‎

Copy file name to clipboardExpand all lines: src/tls_wrap.cc
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,12 @@ void TLSWrap::ClearOut() {
427427
memcpy(buf.base, current, avail);
428428
OnRead(avail, &buf);
429429

430+
// Caveat emptor: OnRead() calls into JS land which can result in
431+
// the SSL context object being destroyed. We have to carefully
432+
// check that ssl_ != nullptr afterwards.
433+
if (ssl_ == nullptr)
434+
return;
435+
430436
read -= avail;
431437
current += avail;
432438
}
Collapse file
+36Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
5+
if (!common.hasCrypto) {
6+
common.skip('missing crypto');
7+
return;
8+
}
9+
10+
const fs = require('fs');
11+
const net = require('net');
12+
const tls = require('tls');
13+
14+
const key = fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem');
15+
const cert = fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem');
16+
const secureContext = tls.createSecureContext({ key, cert });
17+
18+
const server = net.createServer(common.mustCall((conn) => {
19+
const options = { isServer: true, secureContext, server };
20+
const socket = new tls.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+
const options = {
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.