From 854dba649e826cacfd7e682d567a70b388b52184 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 11 Aug 2019 01:31:20 +0200 Subject: [PATCH] http2: stop reading from socket if writes are in progress If a write to the underlying socket finishes asynchronously, that means that we cannot write any more data at that point without waiting for it to finish. If this happens, we should also not be producing any more input. This is part of mitigating CVE-2019-9511/CVE-2019-9517. Backport-PR-URL: https://github.com/nodejs/node/pull/29124 PR-URL: https://github.com/nodejs/node/pull/29122 Reviewed-By: Rich Trott Reviewed-By: James M Snell --- src/node_http2.cc | 19 ++++++++++++++++++- src/node_http2.h | 2 ++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/node_http2.cc b/src/node_http2.cc index 4cedde106fc6b5..eae52c66e10590 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1585,9 +1585,18 @@ void Http2Session::OnStreamAfterWriteImpl(WriteWrap* w, int status, void* ctx) { Http2Session* session = static_cast(ctx); DEBUG_HTTP2SESSION2(session, "write finished with status %d", status); + CHECK_NE(session->flags_ & SESSION_STATE_WRITE_IN_PROGRESS, 0); + session->flags_ &= ~SESSION_STATE_WRITE_IN_PROGRESS; + // Inform all pending writes about their completion. session->ClearOutgoing(status); + if ((session->flags_ & SESSION_STATE_READING_STOPPED) && + nghttp2_session_want_read(session->session_)) { + session->flags_ &= ~SESSION_STATE_READING_STOPPED; + session->stream_->ReadStart(); + } + if (!(session->flags_ & SESSION_STATE_WRITE_SCHEDULED)) { // Schedule a new write if nghttp2 wants to send data. session->MaybeScheduleWrite(); @@ -1627,10 +1636,13 @@ void Http2Session::MaybeScheduleWrite() { } void Http2Session::MaybeStopReading() { + if (flags_ & SESSION_STATE_READING_STOPPED) return; int want_read = nghttp2_session_want_read(session_); DEBUG_HTTP2SESSION2(this, "wants read? %d", want_read); - if (want_read == 0) + if (want_read == 0 || (flags_ & SESSION_STATE_WRITE_IN_PROGRESS)) { + flags_ |= SESSION_STATE_READING_STOPPED; stream_->ReadStop(); + } } // Unset the sending state, finish up all current writes, and reset @@ -1757,6 +1769,8 @@ uint8_t Http2Session::SendPendingData() { chunks_sent_since_last_write_++; + CHECK_EQ(flags_ & SESSION_STATE_WRITE_IN_PROGRESS, 0); + // DoTryWrite may modify both the buffer list start itself and the // base pointers/length of the individual buffers. uv_buf_t* writebufs = *bufs; @@ -1766,8 +1780,11 @@ uint8_t Http2Session::SendPendingData() { return 0; } + flags_ |= SESSION_STATE_WRITE_IN_PROGRESS; + WriteWrap* req = AllocateSend(); if (stream_->DoWrite(req, writebufs, count, nullptr) != 0) { + flags_ &= ~SESSION_STATE_WRITE_IN_PROGRESS; req->Dispose(); } diff --git a/src/node_http2.h b/src/node_http2.h index fc92a74be9d991..bb49bedce3544a 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -384,6 +384,8 @@ enum session_state_flags { SESSION_STATE_CLOSED = 0x4, SESSION_STATE_CLOSING = 0x8, SESSION_STATE_SENDING = 0x10, + SESSION_STATE_WRITE_IN_PROGRESS = 0x20, + SESSION_STATE_READING_STOPPED = 0x40, }; // This allows for 4 default-sized frames with their frame headers