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

Issue 612458 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

Incorrect origin sent with message event in some cases

Reported by bzbar...@mit.edu, May 17 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:49.0) Gecko/20100101 Firefox/49.0

Steps to reproduce the problem:
1. Run the attached web platform test.  Note that this test relies on multiple domains, so really needs to be run in the web platform harness.

What is the expected behavior?
Pass the test.

What went wrong?
Chrome fails the test.  Note that if I do the click() call from EventListener-incumbent-global-subframe.sub.html instead, the test passes in Chrome, which suggests that Chrome is examining the JS stack past the event dispatch to determine the incumbent for the call to the event listener, instead of saving the incumbent with the event listener as it should per spec.

Did this work before? N/A 

Chrome version: 52.0.2729.3 (Official Build) dev (64-bit)  Channel: n/a
OS Version: OS X 10.10
Flash Version: Shockwave Flash 21.0 r0
 
EventListener-test.zip
1.6 KB Download
Cc: jww@chromium.org mkwst@chromium.org
Owner: jochen@chromium.org
Status: Untriaged (was: Unconfirmed)
Thanks for your report. Passing this over to jochen@ for a look.
Project Member

Comment 2 by ClusterFuzz, May 20 2016

Status: Assigned (was: Untriaged)
Project Member

Comment 3 by ClusterFuzz, May 21 2016

Labels: Missing_Severity-1 Missing_Impact-1

Comment 4 by jochen@chromium.org, May 30 2016

Components: Blink>DOM
Boris, thanks for the report. Got a link to the spec handy?

Comment 5 by bzbar...@mit.edu, May 30 2016

The spec is unfortunately in the middle of a rewrite right now, which makes it hard to sort things out from it: some parts got updated but others did not get synced to them.  

But see http://heycam.github.io/webidl/#es-interface step 3 saving the incumbent script.  The section on _invoking_ things lost its uses of callback context temporarily because of the partially-completed rewrite I mention above.  But the idea is that the callback context gets pushed on the incumbent settings stack before you call out into the callback, and that getting the incumbent looks at the incumbent settings stack if the most-recent invocation has no scripted frames.
Project Member

Comment 6 by ClusterFuzz, May 31 2016

Labels: -Missing_Impact-1 -Missing_Severity-1 Missing_Severity-4 Missing_Impact-4
Project Member

Comment 7 by ClusterFuzz, Jun 2 2016

Labels: -Missing_Severity-4 -Missing_Impact-4 Missing_Severity-5 Missing_Impact-5

Comment 8 by f...@chromium.org, Jun 2 2016

Labels: -Missing_Severity-5 -Missing_Impact-5 Security_Impact-Stable Security_Severity-Medium
jochen@, I'm tentatively marking this as a medium-severity bug because it seems like there might be something exploitable (perhaps as part of a chain?) here. please feel free to revise.
Cc: haraken@chromium.org verwa...@chromium.org
I'd say it's low if anything at all.

anyways, I'm kinda lost what the incumbent stack is supposed to be :-/ we only have the ecmascript defined realm stack, and that isn't necessarily updated for calls (e.g. when TCO kicks in)
Project Member

Comment 10 by sheriffbot@chromium.org, Jun 3 2016

Labels: -Pri-2 Pri-1

Comment 11 by bzbar...@mit.edu, Jun 3 2016

Domenic is about to push some updates to the specs that should make it clearer what's going on.  But the basic idea is that you can do cross-origin postMessage and the message event will have the origin of "the thing that called postMessage".  Chrome is deriving this from the Realm stack, but this allows pages to spoof it by setting up async execution of the postMessage call that then happens when some other Realm is on the stack.  That's what this bug report is about.
sorry, i'm still confused. I guess I'll wait for Domenic's updates

Note that we at least no longer randomly walk up the stack since yesterday :)
V8 / blink shouldn't be doing that anymore after jochen's changes, right?

Comment 14 by f...@chromium.org, Jun 3 2016

Labels: -Security_Severity-Medium Security_Severity-Low
Status: Fixed (was: Assigned)
Marking as fixed based on c#13, please verify and update the CL here. If not fixed, please reopen.
Project Member

Comment 16 by sheriffbot@chromium.org, Jun 11 2016

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

Comment 17 by bugdroid1@chromium.org, Jul 14 2016

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

commit 669039d7af8cd85bee16f013e623de830f3aeeac
Author: yukishiino <yukishiino@chromium.org>
Date: Thu Jul 14 03:39:46 2016

binding: Adds layout tests to check origins of window.postMessage

Imports EventListener-incumbent-global tests from w3c/web-platform-tests.
The tests show the current status of support of postMessage's origins.

The original tests are here:
https://github.com/w3c/web-platform-tests/blob/master/dom/events/EventListener-incumbent-global-1.sub.html
https://github.com/w3c/web-platform-tests/blob/master/dom/events/EventListener-incumbent-global-2.sub.html

BUG= 612458 ,622974

Review-Url: https://codereview.chromium.org/2142173003
Cr-Commit-Position: refs/heads/master@{#405428}

[add] https://crrev.com/669039d7af8cd85bee16f013e623de830f3aeeac/third_party/WebKit/LayoutTests/http/tests/dom/EventListener-incumbent-global-1.html
[add] https://crrev.com/669039d7af8cd85bee16f013e623de830f3aeeac/third_party/WebKit/LayoutTests/http/tests/dom/EventListener-incumbent-global-2-expected.txt
[add] https://crrev.com/669039d7af8cd85bee16f013e623de830f3aeeac/third_party/WebKit/LayoutTests/http/tests/dom/EventListener-incumbent-global-2.html
[add] https://crrev.com/669039d7af8cd85bee16f013e623de830f3aeeac/third_party/WebKit/LayoutTests/http/tests/dom/resources/EventListener-incumbent-global-subframe-1.html
[add] https://crrev.com/669039d7af8cd85bee16f013e623de830f3aeeac/third_party/WebKit/LayoutTests/http/tests/dom/resources/EventListener-incumbent-global-subframe-2.html
[add] https://crrev.com/669039d7af8cd85bee16f013e623de830f3aeeac/third_party/WebKit/LayoutTests/http/tests/dom/resources/EventListener-incumbent-global-subsubframe.html
[modify] https://crrev.com/669039d7af8cd85bee16f013e623de830f3aeeac/third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp

Project Member

Comment 18 by sheriffbot@chromium.org, Sep 17 2016

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

Comment 19 by sheriffbot@chromium.org, Oct 1 2016

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

Comment 20 by sheriffbot@chromium.org, Oct 2 2016

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: allpublic

Sign in to add a comment