unbounded popups with postMessage()
Reported by
liveover...@gmail.com,
Apr 15 2018
|
||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.181 Safari/537.36 Steps to reproduce the problem: 1. Visit https://liveoverflow.com/poc/popunder68.html or open the attached .html file 2. Click on popunder 3. A popunder should be created. What is the expected behavior? The second .open() should be blocked by the pop-up blocker. Focus should remain on the opened window. What went wrong? There appear to be two different issues: The first one is about creating a pop-under. When opening a pop-up and a new tab, then closing the tab, returns focus to the main window (this can be tested with disabled pop-up blocker). The second issue is a trick to bypass the pop-up blocker and open multiple pop-ups with a single user interaction. As can be seen in the PoC, it uses .postMessage from within a user interaction event and the message receiver calls .open(). It appears that the "trust" to open a single pop-up is "replicated", and never invalidated, with each postMessage. Tested with: Chrome Version 65.0.3325.181 (Official Build) (64-bit) (mac/windows) Chrome Canary Version 68.0.3397.0 (Official Build) canary (64-bit) (mac) Chromium Version 68.0.3397.0 (Developer Build) (64-bit) (mac) Did this work before? N/A Chrome version: 65.0.3325.181 Channel: stable OS Version: OS X 10.12.6 Flash Version: I'm not the original finder - I recovered this trick from an advertisement pop-under library.
,
Apr 16 2018
+csharrison, avi Who/which team is working on pop unders?
,
Apr 16 2018
Thanks for the report liveoverflow, I was able to repro on M65 Linux. +mustaq,alexmos for issue 161068 which seems related. Bumping priority, we should never allow a site to create an unbounded # of popups.
,
Apr 16 2018
I'm thinking about the first issue, where creating and closing a tab activates the tab. I can understand activating a new tab if the old tab was active, but if a tab in the background spawns a new tab, the new tab shouldn't get activation.
,
Apr 16 2018
Yep, there are two bugs here: 1. Tab shouldn't be able to spawn a new window and new tab, due to gesture consumption. This is the same bug as the "unbounded popups" case. 2. Tab shouldn't be able to get activation on new windows / tabs if it is in the background.
,
Apr 16 2018
Bisected the pop-under regression to https://chromium.googlesource.com/chromium/src/+log/f85f221742f9000dbe462cf15dfc9ff840505e17..1e4271e15bf240edd313611750bcff9db8a8abca Suspecting https://chromium-review.googlesource.com/729130 mustaq: Could you take a look? Not sure if you've already addressed this elsewhere. I have confirmed this is not an issue with UAv2, but it is a shame to have this broken before then.
,
Apr 17 2018
Yikes! The suspect CL of mine should have no-functional-change so I have no clue so far. Can someone confirm if this is really Mac only. My understanding is that it should affect all OSes. csharrison@, any idea? I am traveling now, will look into this next week unless this repros in Linux in which case I can give it a shot this week.
,
Apr 17 2018
It repros on Linux. Sorry, phone posting on a flight.
,
Apr 17 2018
I also believe it affects all OSes. I have tested it on Windows too - see original post: > Chrome Version 65.0.3325.181 (Official Build) (64-bit) (mac/windows)
,
Apr 17 2018
,
Apr 18 2018
Quick update: The "activation" causing the popunder is on the second window.open() which opens the new tab, _not_ closing that tab.
,
Apr 19 2018
BTW, with --disable-popup-blocking, the popunder exploit is not a regression, it seems like we always activate new tabs. I filed issue 834598 for this piece. This bug should stay focused on unbounded popups.
,
Apr 19 2018
I think I may have a fix, I'll take ownership for now.
,
Apr 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/30b90f332312cf85be6754a5e5c136eaa88730c2 commit 30b90f332312cf85be6754a5e5c136eaa88730c2 Author: Charlie Harrison <csharrison@chromium.org> Date: Fri Apr 20 18:38:15 2018 UserActivation: Fix unbounded postMessage copying This CL adds a new static method on Frame that takes a UserGestureToken rather than just a UserGestureToken::Status. The code as-is creates a UserGestureIndicator from a UserGestureToken::Status which mints a new UserGestureToken. This CL instead passes along the existing token instead, which prevents multi-consume. Bug: 833148 Change-Id: Iacc3e3e7f159010d93d829bf1ba764490b19dd61 Reviewed-on: https://chromium-review.googlesource.com/1018427 Commit-Queue: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Mustaq Ahmed <mustaq@chromium.org> Cr-Commit-Position: refs/heads/master@{#552405} [modify] https://crrev.com/30b90f332312cf85be6754a5e5c136eaa88730c2/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc [add] https://crrev.com/30b90f332312cf85be6754a5e5c136eaa88730c2/chrome/test/data/popup_blocker/post-message-popup.html [modify] https://crrev.com/30b90f332312cf85be6754a5e5c136eaa88730c2/third_party/blink/renderer/core/frame/frame.cc [modify] https://crrev.com/30b90f332312cf85be6754a5e5c136eaa88730c2/third_party/blink/renderer/core/frame/frame.h [modify] https://crrev.com/30b90f332312cf85be6754a5e5c136eaa88730c2/third_party/blink/renderer/core/frame/local_dom_window.cc
,
Apr 20 2018
Fixed, will request a merge to M67 on Monday.
,
Apr 23 2018
The NextAction date has arrived: 2018-04-23
,
Apr 23 2018
Verified on the latest Windows canary, requesting merge to M67.
,
Apr 24 2018
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c4d49488712ec8a76ab05e8338c77b41d84743ce commit c4d49488712ec8a76ab05e8338c77b41d84743ce Author: Charlie Harrison <csharrison@chromium.org> Date: Tue Apr 24 15:03:24 2018 UserActivation: Fix unbounded postMessage copying This CL adds a new static method on Frame that takes a UserGestureToken rather than just a UserGestureToken::Status. The code as-is creates a UserGestureIndicator from a UserGestureToken::Status which mints a new UserGestureToken. This CL instead passes along the existing token instead, which prevents multi-consume. Bug: 833148 Change-Id: Iacc3e3e7f159010d93d829bf1ba764490b19dd61 Reviewed-on: https://chromium-review.googlesource.com/1018427 Commit-Queue: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Mustaq Ahmed <mustaq@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#552405}(cherry picked from commit 30b90f332312cf85be6754a5e5c136eaa88730c2) Reviewed-on: https://chromium-review.googlesource.com/1026010 Reviewed-by: Charlie Harrison <csharrison@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#254} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/c4d49488712ec8a76ab05e8338c77b41d84743ce/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc [add] https://crrev.com/c4d49488712ec8a76ab05e8338c77b41d84743ce/chrome/test/data/popup_blocker/post-message-popup.html [modify] https://crrev.com/c4d49488712ec8a76ab05e8338c77b41d84743ce/third_party/blink/renderer/core/frame/frame.cc [modify] https://crrev.com/c4d49488712ec8a76ab05e8338c77b41d84743ce/third_party/blink/renderer/core/frame/frame.h [modify] https://crrev.com/c4d49488712ec8a76ab05e8338c77b41d84743ce/third_party/blink/renderer/core/frame/local_dom_window.cc |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by susan.boorgula@chromium.org
, Apr 15 2018