Skip to content

Commit

Permalink
tls: fix SSL write error handling
Browse files Browse the repository at this point in the history
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/127
Reviewed-By: Evan Lucas <evanlucas@me.com>
  • Loading branch information
addaleax authored and evanlucas committed Jun 12, 2018
1 parent 92d7b6c commit eea2bce
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/stream_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ void ReportWritesToJSStreamListener::OnStreamAfterReqFinished(
AsyncWrap* async_wrap = req_wrap->GetAsyncWrap();
HandleScope handle_scope(env->isolate());
Context::Scope context_scope(env->context());
CHECK(!async_wrap->persistent().IsEmpty());
Local<Object> req_wrap_obj = async_wrap->object();

Local<Value> argv[] = {
Expand Down
4 changes: 3 additions & 1 deletion src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -622,8 +622,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],
Expand Down

0 comments on commit eea2bce

Please sign in to comment.