diff options
| author | Bruce Dawson <brucedawson@chromium.org> | 2020-09-15 02:09:14 +0000 |
|---|---|---|
| committer | Michael Brüning <michael.bruning@qt.io> | 2020-10-28 12:58:07 +0000 |
| commit | 811208e7b603d78c48f7ba081f882b61791443da (patch) | |
| tree | 50dfacd8d114011ec362b69a6cc8377179b2f837 | |
| parent | e5c6b3de8884b86a011c8270726e2d0213861a79 (diff) | |
[Backport] Security bug 1125199
Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/2407065:
Avoid use-after-free
SetNotWaitingForResponse can trigger a message pump which can then free
the object which |this| points to. This use-after-free can be avoided by
not dereferencing |this| after the call, by ensuring that calling
SetNotWaitingForResponse is the last thing done.
Bug: 1125199
Change-Id: Ie1289c93112151978e6daaa1d24326770028c529
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
(cherry picked from commit 4662223422df1b288ab8047808b17eec6795affe)
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
| -rw-r--r-- | chromium/content/browser/web_contents/web_contents_impl.cc | 6 |
1 files changed, 4 insertions, 2 deletions
diff --git a/chromium/content/browser/web_contents/web_contents_impl.cc b/chromium/content/browser/web_contents/web_contents_impl.cc index fbfe1189b65..d63f1bcf78e 100644 --- a/chromium/content/browser/web_contents/web_contents_impl.cc +++ b/chromium/content/browser/web_contents/web_contents_impl.cc @@ -3047,10 +3047,10 @@ void WebContentsImpl::SetNotWaitingForResponse() { return; waiting_for_response_ = false; - if (delegate_) - delegate_->LoadingStateChanged(this, is_load_to_different_document_); for (auto& observer : observers_) observer.DidReceiveResponse(); + if (delegate_) + delegate_->LoadingStateChanged(this, is_load_to_different_document_); } void WebContentsImpl::SendScreenRects() { @@ -4072,6 +4072,8 @@ void WebContentsImpl::ReadyToCommitNavigation( navigation_handle->GetURL(), net::IsCertStatusError(navigation_handle->GetSSLInfo().cert_status)); + // LoadingStateChanged must be called last in case it triggers deletion of + // |this| due to recursive message pumps. SetNotWaitingForResponse(); } |