tls: prevent use-after-free

indutny · indutny · commit 75930bb38c73 · 2015-06-04T23:38:26.000+02:00
* 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 <trev.norris@gmail.com>
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
@@ -305,13 +305,18 @@ TLSSocket.prototype._wrapHandle = function(handle) {
   });
 
   this.on('close', function() {
-    this._destroySSL();
+    // Make sure we are not doing it on OpenSSL's stack
+    setImmediate(destroySSL, this);
     res = null;
   });
 
   return res;
 };
 
+function destroySSL(self) {
+  self._destroySSL();
+}
+
 TLSSocket.prototype._destroySSL = function _destroySSL() {
   if (!this.ssl) return;
   this.ssl.destroySSL();
diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
@@ -337,6 +337,9 @@ void TLSWrap::EncOutCb(WriteWrap* req_wrap, int status) {
     return;
   }
 
+  if (wrap->ssl_ == nullptr)
+    return;
+
   // Commit
   NodeBIO::FromBIO(wrap->enc_out_)->Read(nullptr, wrap->write_size_);
 
@@ -554,6 +557,7 @@ int TLSWrap::DoWrite(WriteWrap* w,
                      size_t count,
                      uv_stream_t* send_handle) {
   CHECK_EQ(send_handle, nullptr);
+  CHECK_NE(ssl_, nullptr);
 
   bool empty = true;
 
@@ -627,6 +631,11 @@ void TLSWrap::OnAfterWriteImpl(WriteWrap* w, void* ctx) {
 void TLSWrap::OnAllocImpl(size_t suggested_size, uv_buf_t* buf, void* ctx) {
   TLSWrap* wrap = static_cast<TLSWrap*>(ctx);
 
+  if (wrap->ssl_ == nullptr) {
+    *buf = uv_buf_init(nullptr, 0);
+    return;
+  }
+
   size_t size = 0;
   buf->base = NodeBIO::FromBIO(wrap->enc_in_)->PeekWritable(&size);
   buf->len = size;
@@ -747,6 +756,10 @@ void TLSWrap::SetVerifyMode(const FunctionCallbackInfo<Value>& args) {
 void TLSWrap::EnableSessionCallbacks(
     const FunctionCallbackInfo<Value>& args) {
   TLSWrap* wrap = Unwrap<TLSWrap>(args.Holder());
+  if (wrap->ssl_ == nullptr) {
+    return wrap->env()->ThrowTypeError(
+        "EnableSessionCallbacks after destroySSL");
+  }
   wrap->enable_session_callbacks();
   NodeBIO::FromBIO(wrap->enc_in_)->set_initial(kMaxHelloLength);
   wrap->hello_parser_.Start(SSLWrap<TLSWrap>::OnClientHello,
diff --git a/test/parallel/test-tls-js-stream.js b/test/parallel/test-tls-js-stream.js
@@ -61,6 +61,7 @@ var server = tls.createServer({
 
     socket.end('hello');
     socket.resume();
+    socket.destroy();
   });
 
   socket.once('close', function() {
