Issue metadata
Sign in to add a comment
|
Security: Universal XSS through bypassing ScopedPageSuspender with closing windows
Reported by
marius.mlynski@gmail.com,
Dec 5 2016
|
|||||||||||||||||||||||
Issue description
VULNERABILITY DETAILS
ScopedPageSuspender works by taking pages from Page::ordinaryPages() and marking them as suspended. When window.close() is called, the following operations are performed:
From /third_party/WebKit/Source/web/ChromeClientImpl.cpp:
----------------
void ChromeClientImpl::closeWindowSoon() {
// Make sure this Page can no longer be found by JS.
m_webView->page()->willBeClosed();
// Make sure that all loading is stopped. Ensures that JS stops executing!
m_webView->mainFrame()->stopLoading();
if (m_webView->client())
m_webView->client()->closeWidgetSoon();
}
----------------
|m_webView->page()->willBeClosed()| removes the associated page from the ordinaryPages set. Therefore, suspenders instantiated later, for example during |m_webView->mainFrame()->stopLoading()|, won't include the closing page. This allows an attacker to circumvent the suspender and perform synchronous loads in unexpected circumstances.
VERSION
Chrome 55.0.2883.75 (Stable)
Chrome 55.0.2883.75 (Beta)
Chrome 56.0.2924.14 (Dev)
Chromium 57.0.2943.0 + Pepper Flash (Release build compiled today)
,
Dec 5 2016
,
Dec 5 2016
Meh. I was hoping to avoid maintaining two sets after https://codereview.chromium.org/2174263002, but it appears to be unavoidable.
,
Dec 6 2016
,
Dec 6 2016
,
Dec 14 2016
,
Dec 15 2016
,
Dec 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0879cb1c4d5e7a53d060bfb7cf7cf9ea05aced72 commit 0879cb1c4d5e7a53d060bfb7cf7cf9ea05aced72 Author: dcheng <dcheng@chromium.org> Date: Thu Dec 22 10:36:49 2016 Make sure pages that are closing but not yet closed are still suspended. BUG= 671102 Review-Url: https://codereview.chromium.org/2580703003 Cr-Commit-Position: refs/heads/master@{#440376} [modify] https://crrev.com/0879cb1c4d5e7a53d060bfb7cf7cf9ea05aced72/third_party/WebKit/Source/core/frame/DOMWindow.cpp [modify] https://crrev.com/0879cb1c4d5e7a53d060bfb7cf7cf9ea05aced72/third_party/WebKit/Source/core/page/FrameTree.cpp [modify] https://crrev.com/0879cb1c4d5e7a53d060bfb7cf7cf9ea05aced72/third_party/WebKit/Source/core/page/Page.cpp [modify] https://crrev.com/0879cb1c4d5e7a53d060bfb7cf7cf9ea05aced72/third_party/WebKit/Source/core/page/Page.h [modify] https://crrev.com/0879cb1c4d5e7a53d060bfb7cf7cf9ea05aced72/third_party/WebKit/Source/core/page/ScopedPageSuspender.cpp [modify] https://crrev.com/0879cb1c4d5e7a53d060bfb7cf7cf9ea05aced72/third_party/WebKit/Source/core/page/ScopedPageSuspender.h [modify] https://crrev.com/0879cb1c4d5e7a53d060bfb7cf7cf9ea05aced72/third_party/WebKit/Source/web/ChromeClientImpl.cpp [modify] https://crrev.com/0879cb1c4d5e7a53d060bfb7cf7cf9ea05aced72/third_party/WebKit/Source/web/tests/ChromeClientImplTest.cpp [modify] https://crrev.com/0879cb1c4d5e7a53d060bfb7cf7cf9ea05aced72/third_party/WebKit/Source/web/tests/WebViewTest.cpp
,
Dec 27 2016
,
Dec 27 2016
,
Dec 27 2016
Note that we already merged a generic fix for these class of UXSS in issue 674203 , but this is still a potential correctness issue.
,
Dec 27 2016
Your change meets the bar and is auto-approved for M56 (branch: 2924)
,
Dec 27 2016
,
Dec 30 2016
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 2 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 2 2017
,
Jan 3 2017
So there's a lot of conflicts; resolving them manually is tricky. The simplest path forward is to merge https://codereview.chromium.org/2526163002, and then this CL, but I'm not sure if that's still considered OK.
,
Jan 3 2017
That sounds fine to me, approving both changes for merge into M56
,
Jan 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fad76601729f6b957274071fb0f1d33072ab72c1 commit fad76601729f6b957274071fb0f1d33072ab72c1 Author: Daniel Cheng <dcheng@chromium.org> Date: Tue Jan 03 21:35:21 2017 Rename blink::Page's load deferral to suspension This CL renames: * ScopedPageLoadDeferrer to ScopedPageSuspender, * blink::Page::defersLoading() to suspended(), and * blink::Page::setDefersLoading() to setSuspended(). blink::setDefersLoading does not only defer loadings, but also suspends all associated ExecutionContext and ActiveDOMObjects. Review-Url: https://codereview.chromium.org/2526163002 Cr-Commit-Position: refs/heads/master@{#434378} (cherry picked from commit 5ad6641ca96fc131e8268f649bc6db71170ad736) BUG= 671102 Review-Url: https://codereview.chromium.org/2611753003 . Cr-Commit-Position: refs/branch-heads/2924@{#655} Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059} [modify] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/core/loader/DocumentLoader.cpp [modify] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp [modify] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/core/loader/FrameLoader.cpp [modify] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/core/loader/NavigationScheduler.cpp [modify] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/core/page/BUILD.gn [modify] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/core/page/ChromeClient.cpp [modify] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/core/page/FocusController.cpp [modify] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/core/page/Page.cpp [modify] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/core/page/Page.h [rename] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/core/page/ScopedPageSuspender.cpp [rename] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/core/page/ScopedPageSuspender.h [modify] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/web/ChromeClientImpl.cpp [modify] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/web/WebViewImpl.cpp [modify] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/web/tests/ChromeClientImplTest.cpp [modify] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/web/tests/WebFrameTest.cpp [modify] https://crrev.com/fad76601729f6b957274071fb0f1d33072ab72c1/third_party/WebKit/Source/web/tests/WebViewTest.cpp
,
Jan 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dc3f2823c1485ab302a2144ec1fcfe37f0e5af50 commit dc3f2823c1485ab302a2144ec1fcfe37f0e5af50 Author: Daniel Cheng <dcheng@chromium.org> Date: Tue Jan 03 21:44:44 2017 Make sure pages that are closing but not yet closed are still suspended. BUG= 671102 Review-Url: https://codereview.chromium.org/2580703003 Cr-Commit-Position: refs/heads/master@{#440376} (cherry picked from commit 0879cb1c4d5e7a53d060bfb7cf7cf9ea05aced72) Review-Url: https://codereview.chromium.org/2616513002 . Cr-Commit-Position: refs/branch-heads/2924@{#656} Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059} [modify] https://crrev.com/dc3f2823c1485ab302a2144ec1fcfe37f0e5af50/third_party/WebKit/Source/core/frame/DOMWindow.cpp [modify] https://crrev.com/dc3f2823c1485ab302a2144ec1fcfe37f0e5af50/third_party/WebKit/Source/core/page/FrameTree.cpp [modify] https://crrev.com/dc3f2823c1485ab302a2144ec1fcfe37f0e5af50/third_party/WebKit/Source/core/page/Page.cpp [modify] https://crrev.com/dc3f2823c1485ab302a2144ec1fcfe37f0e5af50/third_party/WebKit/Source/core/page/Page.h [modify] https://crrev.com/dc3f2823c1485ab302a2144ec1fcfe37f0e5af50/third_party/WebKit/Source/core/page/ScopedPageSuspender.cpp [modify] https://crrev.com/dc3f2823c1485ab302a2144ec1fcfe37f0e5af50/third_party/WebKit/Source/core/page/ScopedPageSuspender.h [modify] https://crrev.com/dc3f2823c1485ab302a2144ec1fcfe37f0e5af50/third_party/WebKit/Source/web/ChromeClientImpl.cpp [modify] https://crrev.com/dc3f2823c1485ab302a2144ec1fcfe37f0e5af50/third_party/WebKit/Source/web/tests/ChromeClientImplTest.cpp [modify] https://crrev.com/dc3f2823c1485ab302a2144ec1fcfe37f0e5af50/third_party/WebKit/Source/web/tests/WebViewTest.cpp
,
Jan 12 2017
,
Jan 12 2017
Hi! The panel decided to reward $7,500 for this bug, and a $1337 bonus for the fix in https://chromium.googlesource.com/chromium/src/+/783e19486c
,
Jan 13 2017
,
Jan 24 2017
,
Jan 25 2017
,
Jan 25 2017
,
Apr 4 2017
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 25 2018
|
||||||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||||||
Comment 1 by och...@chromium.org
, Dec 5 2016Labels: Security_Severity-High Security_Impact-Stable OS-All
Owner: dcheng@chromium.org
Status: Assigned (was: Unconfirmed)