Skip to content

Commit 75930bb

Browse files
committed
tls: prevent use-after-free
* Destroy `SSL*` and friends on a next tick to make sure that we are not doing it in one of the OpenSSL callbacks * Add more checks to the C++ methods that might be invoked during destructor's pending queue cleanup Fix: nodejs/node-v0.x-archive#8780 Fix: #1696 PR-URL: #1702 Reviewed-By: Trevor Norris <[email protected]>
1 parent 5795e83 commit 75930bb

File tree

3 files changed

+20
-1
lines changed

3 files changed

+20
-1
lines changed

lib/_tls_wrap.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -305,13 +305,18 @@ TLSSocket.prototype._wrapHandle = function(handle) {
305305
});
306306

307307
this.on('close', function() {
308-
this._destroySSL();
308+
// Make sure we are not doing it on OpenSSL's stack
309+
setImmediate(destroySSL, this);
309310
res = null;
310311
});
311312

312313
return res;
313314
};
314315

316+
function destroySSL(self) {
317+
self._destroySSL();
318+
}
319+
315320
TLSSocket.prototype._destroySSL = function _destroySSL() {
316321
if (!this.ssl) return;
317322
this.ssl.destroySSL();

src/tls_wrap.cc

+13
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,9 @@ void TLSWrap::EncOutCb(WriteWrap* req_wrap, int status) {
337337
return;
338338
}
339339

340+
if (wrap->ssl_ == nullptr)
341+
return;
342+
340343
// Commit
341344
NodeBIO::FromBIO(wrap->enc_out_)->Read(nullptr, wrap->write_size_);
342345

@@ -554,6 +557,7 @@ int TLSWrap::DoWrite(WriteWrap* w,
554557
size_t count,
555558
uv_stream_t* send_handle) {
556559
CHECK_EQ(send_handle, nullptr);
560+
CHECK_NE(ssl_, nullptr);
557561

558562
bool empty = true;
559563

@@ -627,6 +631,11 @@ void TLSWrap::OnAfterWriteImpl(WriteWrap* w, void* ctx) {
627631
void TLSWrap::OnAllocImpl(size_t suggested_size, uv_buf_t* buf, void* ctx) {
628632
TLSWrap* wrap = static_cast<TLSWrap*>(ctx);
629633

634+
if (wrap->ssl_ == nullptr) {
635+
*buf = uv_buf_init(nullptr, 0);
636+
return;
637+
}
638+
630639
size_t size = 0;
631640
buf->base = NodeBIO::FromBIO(wrap->enc_in_)->PeekWritable(&size);
632641
buf->len = size;
@@ -747,6 +756,10 @@ void TLSWrap::SetVerifyMode(const FunctionCallbackInfo<Value>& args) {
747756
void TLSWrap::EnableSessionCallbacks(
748757
const FunctionCallbackInfo<Value>& args) {
749758
TLSWrap* wrap = Unwrap<TLSWrap>(args.Holder());
759+
if (wrap->ssl_ == nullptr) {
760+
return wrap->env()->ThrowTypeError(
761+
"EnableSessionCallbacks after destroySSL");
762+
}
750763
wrap->enable_session_callbacks();
751764
NodeBIO::FromBIO(wrap->enc_in_)->set_initial(kMaxHelloLength);
752765
wrap->hello_parser_.Start(SSLWrap<TLSWrap>::OnClientHello,

test/parallel/test-tls-js-stream.js

+1
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ var server = tls.createServer({
6161

6262
socket.end('hello');
6363
socket.resume();
64+
socket.destroy();
6465
});
6566

6667
socket.once('close', function() {

0 commit comments

Comments
 (0)