From 84f23d2f12749a146155b52080594c2631836398 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 12 Apr 2018 21:53:59 +0200 Subject: [PATCH] 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 --- src/stream_base.cc | 1 + src/tls_wrap.cc | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/stream_base.cc b/src/stream_base.cc index 3f2e4183291d35..86b56c740103ae 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -388,6 +388,7 @@ void ReportWritesToJSStreamListener::OnStreamAfterReqFinished( StreamBase* stream = static_cast(stream_); Environment* env = stream->stream_env(); AsyncWrap* async_wrap = req_wrap->GetAsyncWrap(); + CHECK(!async_wrap->persistent().IsEmpty()); Local req_wrap_obj = async_wrap->object(); Local argv[] = { diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 7ff49522438d65..f2118b5228a351 100644 --- 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 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],