tls: fix SSL write error handling

Fix an use-after-free bug in the TLS implementation. If we return from `DoWrite()` with an early error, we should not be storing the `WriteWrap` object and complete it again at a later point, when it has already been freed (because of the write error). This issue was reported by Jordan Zebor at F5 Networks, who also helped with investigating this bug and coming up with a reproduction. This fixes CVE-2018-7162. Fixes: https://github.com/nodejs-private/security/issues/189 PR-URL: https://github.com/nodejs-private/node-private/pull/130 Reviewed-By: Evan Lucas <[email protected]>
nodejs · Jun 12, 2018 · 84f23d2 · 84f23d2
1 parent be103eb
commit 84f23d2
Show file tree

Hide file tree

Showing 2 changed files with 4 additions and 1 deletion.
diff --git a/src/stream_base.cc b/src/stream_base.cc
@@ -388,6 +388,7 @@ void ReportWritesToJSStreamListener::OnStreamAfterReqFinished(
   StreamBase* stream = static_cast<StreamBase*>(stream_);
   Environment* env = stream->stream_env();
   AsyncWrap* async_wrap = req_wrap->GetAsyncWrap();
+  CHECK(!async_wrap->persistent().IsEmpty());
   Local<Object> req_wrap_obj = async_wrap->object();
 
   Local<Value> argv[] = {

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
@@ -618,8 +618,10 @@ int TLSWrap::DoWrite(WriteWrap* w,
   if (i != count) {
     int err;
     Local<Value> arg = GetSSLError(written, &err, &error_);
-    if (!arg.IsEmpty())
+    if (!arg.IsEmpty()) {
+      current_write_ = nullptr;
       return UV_EPROTO;
+    }
 
     pending_cleartext_input_.insert(pending_cleartext_input_.end(),
                                     &bufs[i],