I was thinking about 2 alternative solutions:

1. Make new ordinary pages inherit the deferral state from the opener frame's page. Unfortunately, that'd also require updating |m_deferredFrames| on a ScopedPageLoadDeferrer to ensure that new pages are marked as non-deferred later when the deferrer is destroyed. That'd probably require making the class more static, and I'm not sure how to reconcile that with multiple deferrers on the stack. Meh.
2. Ban creating new windows from frames belonging to deferred pages. Popping new pages from behind a modal dialog is a somewhat shady concept, and this is a very simple solution, so if it doesn't break anything, I'd go for it.

I've made an attempt at implementing #2 in https://codereview.chromium.org/2035973002.

Thanks for the report, and the start on a patch!

dcheng@, would you be a good owner for this? If so, PTAL?

or perhaps tasak@, i see you have recently done work in core/page?

Nooooooooo. kinuko@, can someone from the Tokyo loading team investigate this so we can distribute the load for these bugs?

japhet@ or I will be happy to review any changes.

As a matter of cleanup, we probably should make the deferred loading flag a per-process flag instead of a per-page flag. That wouldn't stop about:blank navigations during load deferral, though I think it would stop data: urls?

I was thinking about this, but then I noticed the constructor takes an excluded page as a parameter and I concluded a more fine-grained control over the deferral state might be desirable for some reason(s). data: URIs are loaded asynchronously, so they're subject to deferral, yes.

I don't think we actually ever set an exclusion page (probably vestigial from the WebKit era), so that hopefully isn't a limiting factor.

 :( cc-ing some folks who might be able to investigate it a little further

I feel we could probably land both? Marius's patch seems to work but also feels very specific so I'm afraid there could potentially be other similar issues, while changing like Nate suggests might need to touch a lot more files (as well as might be a bit controversial) and may take a little more time.

Yeah, I think it makes sense to land Marius's patch, then do my idea separately.

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

We have a fix and it is LGTM'd. This is a High severity bug; is there any reason not to land the fix?

I suggest we land it ASAP and merge it back into 52. It's a simple patch so it should be a clean merge.

The band-aid fix by marius is LGTM'ed about 2~3 weeks ago but looks like it's not landed yet-- let me just try CQ now.

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

commit 25a8ae78165cd8725465d26e02a342d7121c42f4
Author: marius.mlynski <marius.mlynski@gmail.com>
Date: Mon Jul 04 06:24:56 2016

Don't allow deferred frames to create new windows.

New pages never defer loads, which makes it difficult for ScopedPageLoadDeferrer
to protect against synchronous loads.

This patch adds a check in ChromeClientImpl::createWindow.

BUG= 616907 

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

[modify] https://crrev.com/25a8ae78165cd8725465d26e02a342d7121c42f4/third_party/WebKit/Source/web/ChromeClientImpl.cpp
[modify] https://crrev.com/25a8ae78165cd8725465d26e02a342d7121c42f4/third_party/WebKit/Source/web/tests/ChromeClientImplTest.cpp

Please mark security bugs as fixed as soon as the fix lands, and before requesting merges.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Before we approve merge to M52, Could you please confirm whether this change is baked/verified in Canary and safe to merge?

+awhalley@, can we take this in for M52 release next week?

OK for M52.

Approving merge to M52 branch 2743 based on comment #23. Please merge ASAP or latest by 4:00 PM PST on Monday, 07/18 so we can take it for next week M52 Stable promotion release.

Also does this require a merge to M53 as well?

+govind@ yes, also needed for M53

Approving merge to M53 branch 2785. Please merge ASAP or latest by 4:00 PM PST on Monday, 07/18. Thank you.

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

commit cbe0b97d5cf56f8eaab3b2fc57b925c69d8f7aa0
Author: Kinuko Yasuda <kinuko@chromium.org>
Date: Sun Jul 17 10:12:01 2016

Don't allow deferred frames to create new windows.

New pages never defer loads, which makes it difficult for ScopedPageLoadDeferrer
to protect against synchronous loads.

This patch adds a check in ChromeClientImpl::createWindow.

BUG= 616907 

Review-Url: https://codereview.chromium.org/2035973002
Cr-Commit-Position: refs/heads/master@{#403640}
(cherry picked from commit 25a8ae78165cd8725465d26e02a342d7121c42f4)

Review URL: https://codereview.chromium.org/2160473002 .

Cr-Commit-Position: refs/branch-heads/2743@{#658}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/cbe0b97d5cf56f8eaab3b2fc57b925c69d8f7aa0/third_party/WebKit/Source/web/ChromeClientImpl.cpp
[modify] https://crrev.com/cbe0b97d5cf56f8eaab3b2fc57b925c69d8f7aa0/third_party/WebKit/Source/web/tests/ChromeClientImplTest.cpp

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

commit 65422dba656b2dc51c4a36e6398d5391ae9ef120
Author: Kinuko Yasuda <kinuko@chromium.org>
Date: Sun Jul 17 10:24:16 2016

Don't allow deferred frames to create new windows.

New pages never defer loads, which makes it difficult for ScopedPageLoadDeferrer
to protect against synchronous loads.

This patch adds a check in ChromeClientImpl::createWindow.

BUG= 616907 

Review-Url: https://codereview.chromium.org/2035973002
Cr-Commit-Position: refs/heads/master@{#403640}
(cherry picked from commit 25a8ae78165cd8725465d26e02a342d7121c42f4)

Review URL: https://codereview.chromium.org/2159643002 .

Cr-Commit-Position: refs/branch-heads/2785@{#172}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/65422dba656b2dc51c4a36e6398d5391ae9ef120/third_party/WebKit/Source/web/ChromeClientImpl.cpp
[modify] https://crrev.com/65422dba656b2dc51c4a36e6398d5391ae9ef120/third_party/WebKit/Source/web/tests/ChromeClientImplTest.cpp

Merged to M52 and M53.

Not sure if tests are compiled for beta/stable, but ChromeClientImplTest.cpp may fail due to a mismatch of arguments provided to WebView::create:

https://chromium.googlesource.com/chromium/src.git/+/52.0.2743.79/third_party/WebKit/Source/web/WebViewImpl.cpp#332

Thanks for the heads-up. We don't seem building webkit_unit_tests for Beta/Stable, but will be watching alerts.

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

commit fdaa78ddc7867123001e1c91e79f1f0e830b7e69
Author: Kinuko Yasuda <kinuko@chromium.org>
Date: Mon Jul 18 15:12:13 2016

Fix build failure for the broken merge on M52/M53 tree

BUG=628975,  616907 
TBR=govind@chromium.org

Review URL: https://codereview.chromium.org/2162463002 .

Cr-Commit-Position: refs/branch-heads/2743@{#661}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/fdaa78ddc7867123001e1c91e79f1f0e830b7e69/third_party/WebKit/Source/web/tests/ChromeClientImplTest.cpp

Nice - $8,000 for this one, Marius.

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