Skip to content

Commit

Permalink
http2: stop reading from socket if writes are in progress
Browse files Browse the repository at this point in the history
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: nodejs#29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax authored and targos committed Aug 15, 2019
1 parent 695e38b commit ba624b6
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
17 changes: 16 additions & 1 deletion src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1542,9 +1542,18 @@ void Http2Session::HandleSettingsFrame(const nghttp2_frame* frame) {
void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) {
Debug(this, "write finished with status %d", status);

CHECK_NE(flags_ & SESSION_STATE_WRITE_IN_PROGRESS, 0);
flags_ &= ~SESSION_STATE_WRITE_IN_PROGRESS;

// Inform all pending writes about their completion.
ClearOutgoing(status);

if ((flags_ & SESSION_STATE_READING_STOPPED) &&
nghttp2_session_want_read(session_)) {
flags_ &= ~SESSION_STATE_READING_STOPPED;
stream_->ReadStart();
}

if (!(flags_ & SESSION_STATE_WRITE_SCHEDULED)) {
// Schedule a new write if nghttp2 wants to send data.
MaybeScheduleWrite();
Expand Down Expand Up @@ -1582,10 +1591,13 @@ void Http2Session::MaybeScheduleWrite() {
}

void Http2Session::MaybeStopReading() {
if (flags_ & SESSION_STATE_READING_STOPPED) return;
int want_read = nghttp2_session_want_read(session_);
Debug(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
Expand Down Expand Up @@ -1716,8 +1728,11 @@ uint8_t Http2Session::SendPendingData() {

chunks_sent_since_last_write_++;

CHECK_EQ(flags_ & SESSION_STATE_WRITE_IN_PROGRESS, 0);
flags_ |= SESSION_STATE_WRITE_IN_PROGRESS;
StreamWriteResult res = underlying_stream()->Write(*bufs, count);
if (!res.async) {
flags_ &= ~SESSION_STATE_WRITE_IN_PROGRESS;
ClearOutgoing(res.err);
}

Expand Down
2 changes: 2 additions & 0 deletions src/node_http2.h
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,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,
};

typedef uint32_t(*get_setting)(nghttp2_session* session,
Expand Down

0 comments on commit ba624b6

Please sign in to comment.