|
| 1 | +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Bruce Dawson <brucedawson@chromium.org> |
| 3 | +Date: Thu, 17 Sep 2020 22:34:58 +0000 |
| 4 | +Subject: Avoid use-after-free |
| 5 | + |
| 6 | +SetNotWaitingForResponse can trigger a message pump which can then free |
| 7 | +the object which |this| points to. This use-after-free can be avoided by |
| 8 | +not dereferencing |this| after the call, by ensuring that calling |
| 9 | +SetNotWaitingForResponse is the last thing done. |
| 10 | + |
| 11 | +(cherry picked from commit e1c5c8442210bccfbc2475c9bc75a9cf99bb259e) |
| 12 | + |
| 13 | +Bug: 1125199 |
| 14 | +Change-Id: Ie1289c93112151978e6daaa1d24326770028c529 |
| 15 | +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2407065 |
| 16 | +Reviewed-by: Alex Moshchuk <alexmos@chromium.org> |
| 17 | +Commit-Queue: Bruce Dawson <brucedawson@chromium.org> |
| 18 | +Cr-Original-Commit-Position: refs/heads/master@{#806839} |
| 19 | +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2416264 |
| 20 | +Reviewed-by: Bruce Dawson <brucedawson@chromium.org> |
| 21 | +Cr-Commit-Position: refs/branch-heads/4240@{#816} |
| 22 | +Cr-Branched-From: f297677702651916bbf65e59c0d4bbd4ce57d1ee-refs/heads/master@{#800218} |
| 23 | + |
| 24 | +diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc |
| 25 | +index 36d84bd8a0a80b9801701c48e863762251a99026..28f2c589e8398f922e155b764870335854a5af7b 100644 |
| 26 | +--- a/content/browser/web_contents/web_contents_impl.cc |
| 27 | ++++ b/content/browser/web_contents/web_contents_impl.cc |
| 28 | +@@ -3386,10 +3386,11 @@ void WebContentsImpl::SetNotWaitingForResponse() { |
| 29 | + return; |
| 30 | + |
| 31 | + waiting_for_response_ = false; |
| 32 | +- if (delegate_) |
| 33 | +- delegate_->LoadingStateChanged(this, is_load_to_different_document_); |
| 34 | + for (auto& observer : observers_) |
| 35 | + observer.DidReceiveResponse(); |
| 36 | ++ |
| 37 | ++ if (delegate_) |
| 38 | ++ delegate_->LoadingStateChanged(this, is_load_to_different_document_); |
| 39 | + } |
| 40 | + |
| 41 | + void WebContentsImpl::SendScreenRects() { |
| 42 | +@@ -4487,6 +4488,8 @@ void WebContentsImpl::ReadyToCommitNavigation( |
| 43 | + : false); |
| 44 | + } |
| 45 | + |
| 46 | ++ // LoadingStateChanged must be called last in case it triggers deletion of |
| 47 | ++ // |this| due to recursive message pumps. |
| 48 | + SetNotWaitingForResponse(); |
| 49 | + } |
| 50 | + |
0 commit comments