Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 484e06d

Browse filesBrowse files
kfarnungaddaleax
authored andcommitted
tls: use after free in tls_wrap
The root cause is that `req_wrap` is created in `StreamBase::Write` and passed to `TLSWrap::DoWrite`. In the TLS case the object gets disposed and replaced with a new instance, but the caller's pointer is never updated. When the `StreamBase::Write` method returns, it returns a pointer to the freed object to the caller. In some cases when the object memory has already been reused an assert is hit in `WriteWrap::SetAllocatedStorage` because the pointer is non-null. PR-URL: #18860 Refs: #18676 Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 9169449 commit 484e06d
Copy full SHA for 484e06d

File tree

Expand file treeCollapse file tree

2 files changed

+18
-16
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+18
-16
lines changed
Open diff view settings
Collapse file

‎src/tls_wrap.cc‎

Copy file name to clipboardExpand all lines: src/tls_wrap.cc
+17-16Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -310,10 +310,12 @@ void TLSWrap::EncOut() {
310310

311311

312312
void TLSWrap::OnStreamAfterWrite(WriteWrap* req_wrap, int status) {
313-
// Report back to the previous listener as well. This is only needed for the
314-
// "empty" writes that are passed through directly to the underlying stream.
315-
if (req_wrap != nullptr)
316-
previous_listener_->OnStreamAfterWrite(req_wrap, status);
313+
if (current_empty_write_ != nullptr) {
314+
WriteWrap* finishing = current_empty_write_;
315+
current_empty_write_ = nullptr;
316+
finishing->Done(status);
317+
return;
318+
}
317319

318320
if (ssl_ == nullptr)
319321
status = UV_ECANCELED;
@@ -579,18 +581,17 @@ int TLSWrap::DoWrite(WriteWrap* w,
579581
// However, if there is any data that should be written to the socket,
580582
// the callback should not be invoked immediately
581583
if (BIO_pending(enc_out_) == 0) {
582-
// We destroy the current WriteWrap* object and create a new one that
583-
// matches the underlying stream, rather than the TLSWrap itself.
584-
585-
// Note: We cannot simply use w->object() because of the "optimized"
586-
// way in which we read persistent handles; the JS object itself might be
587-
// destroyed by w->Dispose(), and the Local<Object> we have is not a
588-
// "real" handle in the sense the V8 is aware of its existence.
589-
Local<Object> req_wrap_obj =
590-
w->GetAsyncWrap()->persistent().Get(env()->isolate());
591-
w->Dispose();
592-
w = underlying_stream()->CreateWriteWrap(req_wrap_obj);
593-
return stream_->DoWrite(w, bufs, count, send_handle);
584+
CHECK_EQ(current_empty_write_, nullptr);
585+
current_empty_write_ = w;
586+
StreamWriteResult res =
587+
underlying_stream()->Write(bufs, count, send_handle);
588+
if (!res.async) {
589+
env()->SetImmediate([](Environment* env, void* data) {
590+
TLSWrap* self = static_cast<TLSWrap*>(data);
591+
self->OnStreamAfterWrite(self->current_empty_write_, 0);
592+
}, this, object());
593+
}
594+
return 0;
594595
}
595596
}
596597

Collapse file

‎src/tls_wrap.h‎

Copy file name to clipboardExpand all lines: src/tls_wrap.h
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ class TLSWrap : public AsyncWrap,
152152
std::vector<uv_buf_t> pending_cleartext_input_;
153153
size_t write_size_;
154154
WriteWrap* current_write_ = nullptr;
155+
WriteWrap* current_empty_write_ = nullptr;
155156
bool write_callback_scheduled_ = false;
156157
bool started_;
157158
bool established_;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.