Skip to content

Commit f64515b

Browse files
addaleaxtargos
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. PR-URL: #29122 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent ba332df commit f64515b

File tree

2 files changed

+18
-1
lines changed

2 files changed

+18
-1
lines changed

src/node_http2.cc

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1542,9 +1542,18 @@ void Http2Session::HandleSettingsFrame(const nghttp2_frame* frame) {
15421542
void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) {
15431543
Debug(this, "write finished with status %d", status);
15441544

1545+
CHECK_NE(flags_ & SESSION_STATE_WRITE_IN_PROGRESS, 0);
1546+
flags_ &= ~SESSION_STATE_WRITE_IN_PROGRESS;
1547+
15451548
// Inform all pending writes about their completion.
15461549
ClearOutgoing(status);
15471550

1551+
if ((flags_ & SESSION_STATE_READING_STOPPED) &&
1552+
nghttp2_session_want_read(session_)) {
1553+
flags_ &= ~SESSION_STATE_READING_STOPPED;
1554+
stream_->ReadStart();
1555+
}
1556+
15481557
if (!(flags_ & SESSION_STATE_WRITE_SCHEDULED)) {
15491558
// Schedule a new write if nghttp2 wants to send data.
15501559
MaybeScheduleWrite();
@@ -1582,10 +1591,13 @@ void Http2Session::MaybeScheduleWrite() {
15821591
}
15831592

15841593
void Http2Session::MaybeStopReading() {
1594+
if (flags_ & SESSION_STATE_READING_STOPPED) return;
15851595
int want_read = nghttp2_session_want_read(session_);
15861596
Debug(this, "wants read? %d", want_read);
1587-
if (want_read == 0)
1597+
if (want_read == 0 || (flags_ & SESSION_STATE_WRITE_IN_PROGRESS)) {
1598+
flags_ |= SESSION_STATE_READING_STOPPED;
15881599
stream_->ReadStop();
1600+
}
15891601
}
15901602

15911603
// Unset the sending state, finish up all current writes, and reset
@@ -1716,8 +1728,11 @@ uint8_t Http2Session::SendPendingData() {
17161728

17171729
chunks_sent_since_last_write_++;
17181730

1731+
CHECK_EQ(flags_ & SESSION_STATE_WRITE_IN_PROGRESS, 0);
1732+
flags_ |= SESSION_STATE_WRITE_IN_PROGRESS;
17191733
StreamWriteResult res = underlying_stream()->Write(*bufs, count);
17201734
if (!res.async) {
1735+
flags_ &= ~SESSION_STATE_WRITE_IN_PROGRESS;
17211736
ClearOutgoing(res.err);
17221737
}
17231738

src/node_http2.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,8 @@ enum session_state_flags {
335335
SESSION_STATE_CLOSED = 0x4,
336336
SESSION_STATE_CLOSING = 0x8,
337337
SESSION_STATE_SENDING = 0x10,
338+
SESSION_STATE_WRITE_IN_PROGRESS = 0x20,
339+
SESSION_STATE_READING_STOPPED = 0x40,
338340
};
339341

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

0 commit comments

Comments
 (0)