Skip to content

Commit 0ce699c

Browse files
addaleaxBethGriggs
authored andcommitted
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: #29123 PR-URL: #29122 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 17357d3 commit 0ce699c

File tree

2 files changed

+18
-1
lines changed

2 files changed

+18
-1
lines changed

src/node_http2.cc

+16-1
Original file line numberDiff line numberDiff line change
@@ -1574,9 +1574,18 @@ void Http2Session::HandleSettingsFrame(const nghttp2_frame* frame) {
15741574
void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) {
15751575
Debug(this, "write finished with status %d", status);
15761576

1577+
CHECK_NE(flags_ & SESSION_STATE_WRITE_IN_PROGRESS, 0);
1578+
flags_ &= ~SESSION_STATE_WRITE_IN_PROGRESS;
1579+
15771580
// Inform all pending writes about their completion.
15781581
ClearOutgoing(status);
15791582

1583+
if ((flags_ & SESSION_STATE_READING_STOPPED) &&
1584+
nghttp2_session_want_read(session_)) {
1585+
flags_ &= ~SESSION_STATE_READING_STOPPED;
1586+
stream_->ReadStart();
1587+
}
1588+
15801589
if (!(flags_ & SESSION_STATE_WRITE_SCHEDULED)) {
15811590
// Schedule a new write if nghttp2 wants to send data.
15821591
MaybeScheduleWrite();
@@ -1616,10 +1625,13 @@ void Http2Session::MaybeScheduleWrite() {
16161625
}
16171626

16181627
void Http2Session::MaybeStopReading() {
1628+
if (flags_ & SESSION_STATE_READING_STOPPED) return;
16191629
int want_read = nghttp2_session_want_read(session_);
16201630
Debug(this, "wants read? %d", want_read);
1621-
if (want_read == 0)
1631+
if (want_read == 0 || (flags_ & SESSION_STATE_WRITE_IN_PROGRESS)) {
1632+
flags_ |= SESSION_STATE_READING_STOPPED;
16221633
stream_->ReadStop();
1634+
}
16231635
}
16241636

16251637
// Unset the sending state, finish up all current writes, and reset
@@ -1746,8 +1758,11 @@ uint8_t Http2Session::SendPendingData() {
17461758

17471759
chunks_sent_since_last_write_++;
17481760

1761+
CHECK_EQ(flags_ & SESSION_STATE_WRITE_IN_PROGRESS, 0);
1762+
flags_ |= SESSION_STATE_WRITE_IN_PROGRESS;
17491763
StreamWriteResult res = underlying_stream()->Write(*bufs, count);
17501764
if (!res.async) {
1765+
flags_ &= ~SESSION_STATE_WRITE_IN_PROGRESS;
17511766
ClearOutgoing(res.err);
17521767
}
17531768

src/node_http2.h

+2
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,8 @@ enum session_state_flags {
334334
SESSION_STATE_CLOSED = 0x4,
335335
SESSION_STATE_CLOSING = 0x8,
336336
SESSION_STATE_SENDING = 0x10,
337+
SESSION_STATE_WRITE_IN_PROGRESS = 0x20,
338+
SESSION_STATE_READING_STOPPED = 0x40,
337339
};
338340

339341
typedef uint32_t(*get_setting)(nghttp2_session* session,

0 commit comments

Comments
 (0)