New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security

Blocked on:
issue 674203



Sign in to add a comment
link

Issue 671102: 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)
 
exploit.zip
3.0 KB Download

Comment 1 by och...@chromium.org, Dec 5 2016

Components: Blink
Labels: Security_Severity-High Security_Impact-Stable OS-All
Owner: dcheng@chromium.org
Status: Assigned (was: Unconfirmed)
dcheng, would you mind helping with finding the right owner for this? Thanks.

Comment 2 by dcheng@chromium.org, Dec 5 2016

Cc: haraken@chromium.org esprehn@chromium.org alex...@chromium.org

Comment 3 by dcheng@chromium.org, Dec 5 2016

Status: Started (was: Assigned)
Meh. I was hoping to avoid maintaining two sets after https://codereview.chromium.org/2174263002, but it appears to be unavoidable.

Comment 4 by sheriffbot@chromium.org, Dec 6 2016

Project Member
Labels: M-55

Comment 5 by sheriffbot@chromium.org, Dec 6 2016

Project Member
Labels: Pri-1

Comment 6 by dcheng@chromium.org, Dec 14 2016

Blockedon: 674203

Comment 7 by mmoroz@chromium.org, Dec 15 2016

Cc: mmoroz@chromium.org

Comment 8 by bugdroid1@chromium.org, Dec 22 2016

Project Member
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

Comment 9 by dcheng@chromium.org, Dec 27 2016

Status: Fixed (was: Started)

Comment 10 by dcheng@chromium.org, Dec 27 2016

Labels: Merge-Request-56

Comment 11 by dcheng@chromium.org, 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.

Comment 12 by dimu@chromium.org, Dec 27 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)

Comment 13 by sheriffbot@chromium.org, Dec 27 2016

Project Member
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify

Comment 14 by sheriffbot@chromium.org, Dec 30 2016

Project Member
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

Comment 15 by sheriffbot@chromium.org, Jan 2 2017

Project Member
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

Comment 16 by awhalley@chromium.org, Jan 2 2017

Labels: reward-topanel

Comment 17 by dcheng@chromium.org, 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.

Comment 18 by bustamante@google.com, Jan 3 2017

That sounds fine to me, approving both changes for merge into M56

Comment 19 by bugdroid1@chromium.org, Jan 3 2017

Project Member
Labels: -merge-approved-56 merge-merged-2924
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

Comment 20 by bugdroid1@chromium.org, Jan 3 2017

Project Member
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

Comment 21 by awhalley@chromium.org, Jan 12 2017

Labels: -reward-topanel reward-unpaid reward-8837

Comment 22 by awhalley@chromium.org, 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

Comment 23 by awhalley@chromium.org, Jan 13 2017

Labels: -Hotlist-Merge-Approved

Comment 24 by awhalley@chromium.org, Jan 24 2017

Labels: -M-55 Release-0-M56 M-56

Comment 25 by awhalley@chromium.org, Jan 25 2017

Labels: CVE-2017-5007

Comment 26 by awhalley@chromium.org, Jan 25 2017

Labels: -reward-unpaid reward-inprocess

Comment 27 by sheriffbot@chromium.org, Apr 4 2017

Project Member
Labels: -Restrict-View-SecurityNotify allpublic
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

Comment 28 by awhalley@chromium.org, Apr 25 2018

Labels: CVE_description-submitted

Sign in to add a comment