Commit 22b050d8 authored by Zhongyi Shi's avatar Zhongyi Shi Committed by Commit Bot

Fix use after free issue in pending_stream_map_.

When QuicSession::ClosePendingStream, before remove the pending stream
from the pending_stream_map_, QuicSession::SendRstStream is invoked to
send a reset acknowledgement to the peer. In Chromium,
QuicChromiumClientSession overrides SendRstStream method, and calls into
GetOrCreateDynamicStreamImpl() to close the pending stream as well as
remove from pending_stream_map_. QuicSession holding the already deleted
iterator will attempt to remove the entry again.

This change adds iterator check right before removing the entry to avoid
use after free issue.

Bug: 918849, 918890
Change-Id: I27b7a4433d6783e4484a5dcb49446d89e997b8bf
Reviewed-on: https://chromium-review.googlesource.com/c/1394920
Commit-Queue: Zhongyi Shi <zhongyi@chromium.org>
Reviewed-by: default avatarNick Harper <nharper@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619806}
parent 5b869185
...@@ -694,15 +694,18 @@ void QuicSession::CloseStreamInner(QuicStreamId stream_id, bool locally_reset) { ...@@ -694,15 +694,18 @@ void QuicSession::CloseStreamInner(QuicStreamId stream_id, bool locally_reset) {
void QuicSession::ClosePendingStream(QuicStreamId stream_id) { void QuicSession::ClosePendingStream(QuicStreamId stream_id) {
QUIC_DVLOG(1) << ENDPOINT << "Closing stream " << stream_id; QUIC_DVLOG(1) << ENDPOINT << "Closing stream " << stream_id;
auto it = pending_stream_map_.find(stream_id); if (pending_stream_map_.find(stream_id) == pending_stream_map_.end()) {
if (it == pending_stream_map_.end()) {
QUIC_BUG << ENDPOINT << "Stream is already closed: " << stream_id; QUIC_BUG << ENDPOINT << "Stream is already closed: " << stream_id;
return; return;
} }
SendRstStream(stream_id, QUIC_RST_ACKNOWLEDGEMENT, 0); SendRstStream(stream_id, QUIC_RST_ACKNOWLEDGEMENT, 0);
pending_stream_map_.erase(it); // The pending stream may have been deleted and removed during SendRstStream.
// Remove the stream from pending stream map iff it is still in the map.
if (pending_stream_map_.find(stream_id) != pending_stream_map_.end()) {
pending_stream_map_.erase(stream_id);
}
--num_dynamic_incoming_streams_; --num_dynamic_incoming_streams_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment