Skip to content

Commit 854dba6

Browse files
addaleaxBethGriggs
authored andcommittedAug 15, 2019
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: #29124 PR-URL: #29122 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent a319168 commit 854dba6

File tree

2 files changed

+20
-1
lines changed

2 files changed

+20
-1
lines changed
 

‎src/node_http2.cc

+18-1
Original file line numberDiff line numberDiff line change
@@ -1585,9 +1585,18 @@ void Http2Session::OnStreamAfterWriteImpl(WriteWrap* w, int status, void* ctx) {
15851585
Http2Session* session = static_cast<Http2Session*>(ctx);
15861586
DEBUG_HTTP2SESSION2(session, "write finished with status %d", status);
15871587

1588+
CHECK_NE(session->flags_ & SESSION_STATE_WRITE_IN_PROGRESS, 0);
1589+
session->flags_ &= ~SESSION_STATE_WRITE_IN_PROGRESS;
1590+
15881591
// Inform all pending writes about their completion.
15891592
session->ClearOutgoing(status);
15901593

1594+
if ((session->flags_ & SESSION_STATE_READING_STOPPED) &&
1595+
nghttp2_session_want_read(session->session_)) {
1596+
session->flags_ &= ~SESSION_STATE_READING_STOPPED;
1597+
session->stream_->ReadStart();
1598+
}
1599+
15911600
if (!(session->flags_ & SESSION_STATE_WRITE_SCHEDULED)) {
15921601
// Schedule a new write if nghttp2 wants to send data.
15931602
session->MaybeScheduleWrite();
@@ -1627,10 +1636,13 @@ void Http2Session::MaybeScheduleWrite() {
16271636
}
16281637

16291638
void Http2Session::MaybeStopReading() {
1639+
if (flags_ & SESSION_STATE_READING_STOPPED) return;
16301640
int want_read = nghttp2_session_want_read(session_);
16311641
DEBUG_HTTP2SESSION2(this, "wants read? %d", want_read);
1632-
if (want_read == 0)
1642+
if (want_read == 0 || (flags_ & SESSION_STATE_WRITE_IN_PROGRESS)) {
1643+
flags_ |= SESSION_STATE_READING_STOPPED;
16331644
stream_->ReadStop();
1645+
}
16341646
}
16351647

16361648
// Unset the sending state, finish up all current writes, and reset
@@ -1757,6 +1769,8 @@ uint8_t Http2Session::SendPendingData() {
17571769

17581770
chunks_sent_since_last_write_++;
17591771

1772+
CHECK_EQ(flags_ & SESSION_STATE_WRITE_IN_PROGRESS, 0);
1773+
17601774
// DoTryWrite may modify both the buffer list start itself and the
17611775
// base pointers/length of the individual buffers.
17621776
uv_buf_t* writebufs = *bufs;
@@ -1766,8 +1780,11 @@ uint8_t Http2Session::SendPendingData() {
17661780
return 0;
17671781
}
17681782

1783+
flags_ |= SESSION_STATE_WRITE_IN_PROGRESS;
1784+
17691785
WriteWrap* req = AllocateSend();
17701786
if (stream_->DoWrite(req, writebufs, count, nullptr) != 0) {
1787+
flags_ &= ~SESSION_STATE_WRITE_IN_PROGRESS;
17711788
req->Dispose();
17721789
}
17731790

‎src/node_http2.h

+2
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,8 @@ enum session_state_flags {
384384
SESSION_STATE_CLOSED = 0x4,
385385
SESSION_STATE_CLOSING = 0x8,
386386
SESSION_STATE_SENDING = 0x10,
387+
SESSION_STATE_WRITE_IN_PROGRESS = 0x20,
388+
SESSION_STATE_READING_STOPPED = 0x40,
387389
};
388390

389391
// This allows for 4 default-sized frames with their frame headers

0 commit comments

Comments
 (0)
Please sign in to comment.