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: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: URL spoofing via crafted flash file and UI overlay

Reported by ma7h1a...@gmail.com, Aug 3 2017

Issue description

URL spoofing via crafted flash file and UI overlay
2017.8.3


AFFECTED PRODUCTS
--------------------
chrome 59.0.3071.115


DESCRIPTION
--------------------
when ActionScript use navigateToURL function to open a new window
the URL updates before actual navigation
i guess it's similar to https://bugs.chromium.org/p/chromium/issues/detail?id=660498 (PDF)

when a new window with no toolbar is opened,it could overlay the fullscreen notification
it looks like https://bugs.chromium.org/p/chromium/issues/detail?id=565760

but when i only use flash to spoofing , as i write something into the spoofing page
the address turned to about:blank
so i combine the following two methods to finish this attack

1.spoofing to account.google.com:81 for 8 seconds (or longer)
make a fake page which tells user that there is a network error.
2.user click the reload button
then I draw fake address bar. open a new window to overlay the fullscreen notification


PoC
--------------------
online example

http://www.math1as.com/spoofing_poc.html

poc.zip for your local httpserver

attack1.gif shows spoofing only use method 1

attack2.gif shows the total attack

SOLUTION
--------------------
follow  issue 660498  to fix the flash problem
cancel full-screen mode when open a new window


CREDIT
--------------------
This vulnerability was discovered by mathiaswu of Tencent's Xuanwu Lab.
 
poc.zip
48.6 KB Download
attack1.gif
350 KB View Download
attack2.gif
443 KB View Download
evil.as
409 bytes View Download
what's more , when a new window is opened during full-screen mode , the taskbar in bottom appears so that user can't realized that he is in full-screen mode
so,this is the .webm video for the total attack
attack.webm
301 KB View Download
I'm sorry to bother, but is there anyone to handle this issue?
Components: UI>Browser>Navigation
I suspect this will indeed be quite similar to  Issue 660498  in root cause.
Components: UI>Browser>Omnibox Internals>Plugins>Flash
Labels: Security_Severity-Medium Security_Impact-Stable M-62 Pri-2
Owner: creis@chromium.org
Status: Assigned (was: Unconfirmed)
Labels: OS-Windows

Comment 7 by creis@chromium.org, Aug 8 2017

Cc: a...@chromium.org dcheng@chromium.org alex...@chromium.org foolip@chromium.org
Components: Blink>Fullscreen
Sounds like this is a fullscreen bug, if we're letting a popup show up over fullscreen mode and hide the fullscreen notification.  Shouldn't we exit fullscreen mode if a popup is shown?

Note that attack2.gif and the video in comment 2 are the concerning ones.  attack1.gif shows expected behavior, where we reset the URL bar to about:blank if an opener window tries to modify the page while a slow URL is loading (per  issue 9682 ).

Comment 8 by a...@chromium.org, Aug 8 2017

We drop out of HTML5 fullscreen. I thought that the code that I use to do that would kick me out of Flash fullscreen. Is this using Flash fullscreen?

Comment 9 by creis@chromium.org, Aug 8 2017

Cc: creis@chromium.org
Owner: a...@chromium.org
The attached poc.zip has a modified version of Chrome's error page encoded in the attack page.  It seems to be using HTML5 fullscreen to launch the attack.  (Flash only appears to be used to open the URL in a new tab, though I'm not sure why that's needed.  It would work with slow links that open in a new tab as well, even without Flash.)

Here's the relevant code snippet:

<button id="reload-button" class="blue-button text-button" onclick="document.body.webkitRequestFullscreen();document.body.innerHTML='<img src=111.jpg width=\'101.5%\' height=\'6%\' style=\'margin-left:-8px;margin-top:-6px\' /><div style=\'background-color:white;width:102.5%;margin-left:-20px;height:96.5%;border-style:none;\'><br><br><h2>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;oh! this is a fake google</h2></div>';t=window.open('','','height=100,width=400,top=0,left=800,toolbar=no,menubar=no,scrollbars=no, resizable=no,location=no, status=no');t.document.write('overlay the fullscreen notification')">Reload</button>

That appears to work in a simpler test page, showing a popup over fullscreen.  Avi, do you want to take a look at that?

Comment 10 by a...@chromium.org, Aug 8 2017

Re the video in comment 2, that's not a JavaScript dialog. It's a popup.
Screen Shot 2017-08-08 at 2.39.39 PM.png
21.6 KB View Download
Right, we're discussing popups, not dialogs.  :)

Comment 12 by a...@chromium.org, Aug 8 2017

Just realized that.

The easy answer is, if a page shows a popup, that should kick it out of fullscreen. Is that another Blink Intent?

The other answer would be to debug window ordering in Views, but that's not anything I know about.

Comment 13 by a...@chromium.org, Aug 8 2017

In my Intent re dialogs exiting fullscreen, Jochen wrote (https://groups.google.com/a/chromium.org/d/msg/blink-dev/waIh70rrNKM/4h_da0DwBgAJ):

"I changed window.open() exiting fullscreen a long time ago, and I feel like doing the same for dialogs is in line with this."

This _should_ already be kicking pages out of fullscreen. What's going on?

Comment 14 by a...@chromium.org, Aug 8 2017

FYI that change from Jochen is https://codereview.chromium.org/27599002 .

Comment 15 by a...@chromium.org, Aug 8 2017

This change from Jochen fails on Windows, but works fine on the Mac. The change is deep in Blink, and I don't know why it fails.

Comment 16 by a...@chromium.org, Aug 8 2017

Is there someone good with Blink who can investigate why the fullscreen exit isn't working on Windows?
Project Member

Comment 17 by sheriffbot@chromium.org, Aug 9 2017

Labels: -Pri-2 Pri-1
Labels: OS-Linux
I did some debugging. I'm not sure why this doesn't repro on Mac, but it definitely repros on Linux as well. What I'm seeing is that FullyExitFullscreen takes this early return: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/fullscreen/Fullscreen.cpp?rcl=4f9d2469ba713829aaa2e62fdd766a64cb5965d3&l=620

I believe this is because going into fullscreen mode is an async IPC, while creating a new window is a sync IPC.

One way to address this is to move the checks into the browser like https://chromium-review.googlesource.com/c/606987 proposes: even though fullscreen is an async IPC, and creating a new window is a sync IPC, this gives us the ordering guarantee that the fullscreen IPC arrives (and is processed before) the create new window IPC.

Comment 19 by a...@chromium.org, Aug 10 2017

Here's an interesting twist. If you fully revert https://codereview.chromium.org/27599002 and do nothing more, showing a popup _still_ drops the page out of fullscreen on the Mac. The activation of the popup calls Browser::TabDeactivated, which calls ExclusiveAccessManager::OnTabDeactivated, which does the fullscreen drop.

I'm not clear what's going on on Windows, but I'll look into still doing it in WebContents.

Comment 20 by a...@chromium.org, Aug 10 2017

And in fact, even if you don't revert https://codereview.chromium.org/27599002 but instrument fullscreen, it's not the code in Blink that kicks Chrome out of fullscreen, but actually that codepath through Browser::TabDeactivated.

My conclusion is that the code in https://codereview.chromium.org/27599002, while perhaps effective in the past and in the layout tests, has zero effect on modern Chrome for the user when it comes to fullscreen and popups.

My plan is to fully revert it and reimplement it in WebContents.

Comment 21 by a...@chromium.org, Aug 10 2017

OTOH, the WPTs "fully-exit-fullscreen*" rely on window.open immediately kicking a page out of fullscreen synchronously from their point of view. Note that they escape the race by waiting for an "onfullscreenchange" handler callback before calling window.open.

Which means that to not break the WPTs we need to keep the existing code in Blink, we need to keep the existing code in Blink and add more in Chromium.

Comment 22 by a...@chromium.org, Aug 10 2017

Specifically, these tests were added to test Fullscreen::fullyExitFullscreen() in https://chromium.googlesource.com/chromium/src/+/5dec13adba85193629911ce63d437cad5f2c3b88 and I'm a loath to remove them.

Philip, thoughts?
This is the tests in https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/fullscreen/model/, right?

These are actually not part of web-platform-tests, because when I wrote them the window.open behavior wasn't mentioned in the spec (still isn't) and I think it also didn't work that way in all implementations. There's a FIXME in fully-exit-fullscreen.js lamenting the situation.

So, it would be fine to change these tests to cover whatever the new behavior is. Or will this be a case where content_shell doesn't work the same as chrome, so that it actually can't be tested with LayoutTests?

Comment 24 by a...@chromium.org, Aug 11 2017

I'm not sure how fullscreen works in content shell, only that when I removed the Blink implementation those tests were failing. Because they use onfullscreenchange, they _should_ work with the new implementation, though. I can take a look.

(There would be a new test; see https://chromium-review.googlesource.com/c/606987 to see what I have in mind.)
That looks OK to me. Having something spec'd and tested in wpt would be even nicer, but that's not happening in a hurry, and also not a high priority I think.

Comment 26 by a...@chromium.org, Aug 11 2017

I just confirmed that this fixes the problem on Windows in a local build. Philip, if you're OK with switching to a browser-side fix, I'll rev the CL and send it out for review.

Thank you, all!
Project Member

Comment 27 by sheriffbot@chromium.org, Aug 26 2017

avi: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

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

Comment 28 by bugdroid1@chromium.org, Aug 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ba3b1b344017bbf36283464b51014fad15c2f3f4

commit ba3b1b344017bbf36283464b51014fad15c2f3f4
Author: Avi Drissman <avi@chromium.org>
Date: Tue Aug 29 18:16:13 2017

If a page shows a popup, end fullscreen.

This was implemented in Blink r159834, but it is susceptible
to a popup/fullscreen race. This CL reverts that implementation
and re-implements it in WebContents.

BUG= 752003 
TEST=WebContentsImplBrowserTest.PopupsFromJavaScriptEndFullscreen

Change-Id: Ia345cdeda273693c3231ad8f486bebfc3d83927f
Reviewed-on: https://chromium-review.googlesource.com/606987
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Philip J├Ągenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498171}
[modify] https://crrev.com/ba3b1b344017bbf36283464b51014fad15c2f3f4/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/ba3b1b344017bbf36283464b51014fad15c2f3f4/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/ba3b1b344017bbf36283464b51014fad15c2f3f4/content/browser/web_contents/web_contents_impl_browsertest.cc
[modify] https://crrev.com/ba3b1b344017bbf36283464b51014fad15c2f3f4/third_party/WebKit/Source/core/page/ChromeClientImpl.cpp

Comment 29 by a...@chromium.org, Aug 29 2017

Status: Fixed (was: Assigned)
So that's landed. Marking fixed; can the OP confirm on what I imagine will be the 3200 canary?
Project Member

Comment 30 by sheriffbot@chromium.org, Aug 30 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: reward-topanel
Labels: -reward-topanel reward-unpaid reward-1000
*** Boilerplate reminders! ***
Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************
Many thanks for the report ma7h1as.l@! The VRP Panel decided to award $1,000 for this bug.
Labels: -reward-unpaid reward-inprocess
Thanks , please credit as Wenxu Wu of Tencent's Xuanwu Lab when you are ready to release a new version google.
Project Member

Comment 36 by sheriffbot@chromium.org, Sep 15 2017

Labels: Merge-Request-62
Project Member

Comment 37 by sheriffbot@chromium.org, Sep 15 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: M62 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-62
Already in M62
Labels: Release-0-M62
Labels: CVE-2017-15386
Project Member

Comment 41 by sheriffbot@chromium.org, Dec 6 2017

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
Labels: CVE_description-submitted

Sign in to add a comment