New issue
Advanced search Search tips

Issue 733330 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 756089



Sign in to add a comment

subresource_filter: pages with activation should have a more aggressive popup blocker.

Project Member Reported by csharrison@chromium.org, Jun 14 2017

Issue description

The logic for pages without activation is to only allow popups with a user gesture. A common way around that is "clickjacking" where the site triggers a popup on any click (not just a link).

Sites with activation should strip the user gesture bit.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 14 2017

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

commit 8fe04b042e754d0d585ad27f295d272069484db3
Author: Charles Harrison <csharrison@chromium.org>
Date: Wed Jun 14 22:25:30 2017

[subresource_filter] Make the popup blocker more robust

Previously we had just been hooking in at CanCreateNewWindow, but we
were missing the OpenURLFromTab case. This CL aligns the
subresource_filter to be applied wherever the popup blocker is.

Bug:  733330 
Change-Id: Idc6f9f010f1db1c9cca5037e6e901436ec5b0152
Reviewed-on: https://chromium-review.googlesource.com/532035
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479525}
[modify] https://crrev.com/8fe04b042e754d0d585ad27f295d272069484db3/chrome/browser/android/tab_web_contents_delegate_android.cc
[modify] https://crrev.com/8fe04b042e754d0d585ad27f295d272069484db3/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/8fe04b042e754d0d585ad27f295d272069484db3/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
[modify] https://crrev.com/8fe04b042e754d0d585ad27f295d272069484db3/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc
[modify] https://crrev.com/8fe04b042e754d0d585ad27f295d272069484db3/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h
[modify] https://crrev.com/8fe04b042e754d0d585ad27f295d272069484db3/chrome/browser/ui/browser.cc
[add] https://crrev.com/8fe04b042e754d0d585ad27f295d272069484db3/chrome/test/data/subresource_filter/window_open_spoof_click.html

Project Member

Comment 2 by bugdroid1@chromium.org, Jun 15 2017

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

commit 794e35f1e8da4da3307773d6a09512e5393e93f2
Author: Charles Harrison <csharrison@chromium.org>
Date: Thu Jun 15 18:33:07 2017

[subresource_filter] Gate stronger popup blocking behind an experiment param

The stronger popup blocker is very aggressive, and it would be wise to
gate it behind a separate finch param, so we can selectively enable it
under certain conditions (e.g. only after a phishing interstitial).

This CL disables it by default, and does not add it to any preset, though
that may be subject to change in the near future.

Bug:  733330 
Change-Id: Icf097d9e3a4863f2417a8804568285328af62bf2
Reviewed-on: https://chromium-review.googlesource.com/537033
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479764}
[modify] https://crrev.com/794e35f1e8da4da3307773d6a09512e5393e93f2/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
[modify] https://crrev.com/794e35f1e8da4da3307773d6a09512e5393e93f2/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc
[modify] https://crrev.com/794e35f1e8da4da3307773d6a09512e5393e93f2/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h
[modify] https://crrev.com/794e35f1e8da4da3307773d6a09512e5393e93f2/components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.cc
[modify] https://crrev.com/794e35f1e8da4da3307773d6a09512e5393e93f2/components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h
[modify] https://crrev.com/794e35f1e8da4da3307773d6a09512e5393e93f2/components/subresource_filter/core/browser/subresource_filter_features.cc
[modify] https://crrev.com/794e35f1e8da4da3307773d6a09512e5393e93f2/components/subresource_filter/core/browser/subresource_filter_features.h
[modify] https://crrev.com/794e35f1e8da4da3307773d6a09512e5393e93f2/components/subresource_filter/core/browser/subresource_filter_features_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 21 2017

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

commit bd8675985355867e8fdd43f23b207bf5a091e231
Author: Charles Harrison <csharrison@chromium.org>
Date: Wed Jun 21 04:55:43 2017

Thread triggering event info into OpenURLParams

The OpenURLParams has knowledge of user_gesture, which is great to feed
into the existing popup blocking heuristics. However, it is very common
for sites to "steal" a user gesture to use for script-initiated
(click-jacking) popups / navigations.

This CL threads an additional bit of information through via IPC to
the OpenURLParams: the trusted state of the JS triggering event which led to
this navigation request.

Docs for Event.isTrusted can be found here:
https://developer.mozilla.org/en-US/docs/Web/API/Event/isTrusted

This extra bit can be used to explore additional popup blocking heuristics,
and is planned to be used in conjunction with the subresource_filter
component.

Bug:  733330 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Iac45ed4800989b45a006483b5e6a6b943e6c5af9
Reviewed-on: https://chromium-review.googlesource.com/539898
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481112}
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/content/browser/DEPS
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/content/browser/frame_host/navigator.h
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/content/browser/frame_host/navigator_impl.h
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/content/browser/security_exploit_browsertest.cc
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/content/common/DEPS
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/content/common/frame_messages.h
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/content/public/browser/page_navigator.h
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/content/renderer/render_frame_impl.h
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/content/renderer/render_frame_proxy.cc
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/third_party/WebKit/Source/core/frame/LocalFrameClient.h
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/third_party/WebKit/Source/core/loader/EmptyClients.cpp
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/third_party/WebKit/Source/core/loader/FrameLoader.h
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/third_party/WebKit/Source/web/LocalFrameClientImpl.h
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/third_party/WebKit/public/web/WebFrameClient.h
[add] https://crrev.com/bd8675985355867e8fdd43f23b207bf5a091e231/third_party/WebKit/public/web/WebTriggeringEventInfo.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 21 2017

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

commit f072428c5dbc5d7a1b5a40afe521324c2a08c7f3
Author: Charles Harrison <csharrison@chromium.org>
Date: Wed Jun 21 19:53:33 2017

[subresource_filter] Weaker popup blocker for OpenURLFromTab case

It turns out this more extreme form of blocking applies to lots of true
user initiated navigations like shift-click. The normal popup blocker is
100% guarded via the user_gesture bit only.

This CL plumbs the OpenURLParams through to the subresource filter but
only checks for existence for now. This will make it simpler for a
followup [1] to add a param to OpenURLParams like isTrusted which could
parallel the isTrusted bit in JS events.

[1]: https://chromium-review.googlesource.com/c/539898/

Bug:  733330 
Change-Id: I46838cd6da799fff585b6e860ce0534799729f1e
Reviewed-on: https://chromium-review.googlesource.com/539896
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481282}
[modify] https://crrev.com/f072428c5dbc5d7a1b5a40afe521324c2a08c7f3/chrome/browser/android/tab_web_contents_delegate_android.cc
[modify] https://crrev.com/f072428c5dbc5d7a1b5a40afe521324c2a08c7f3/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/f072428c5dbc5d7a1b5a40afe521324c2a08c7f3/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
[modify] https://crrev.com/f072428c5dbc5d7a1b5a40afe521324c2a08c7f3/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc
[modify] https://crrev.com/f072428c5dbc5d7a1b5a40afe521324c2a08c7f3/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h
[modify] https://crrev.com/f072428c5dbc5d7a1b5a40afe521324c2a08c7f3/chrome/browser/ui/browser.cc
[modify] https://crrev.com/f072428c5dbc5d7a1b5a40afe521324c2a08c7f3/components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.cc
[modify] https://crrev.com/f072428c5dbc5d7a1b5a40afe521324c2a08c7f3/components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 22 2017

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

commit cbbf1c73bae64dc528a052c38830b6e6956db048
Author: Charles Harrison <csharrison@chromium.org>
Date: Thu Jun 22 16:51:36 2017

[subresource_filter] Block popups from navigations with false Event.isTrusted

This will enable blocking e.g. spoofed shift-clicks.

Bug:  733330 
Change-Id: I50de45664b2f95bd9743d14c23d43fec4782eb0f
Reviewed-on: https://chromium-review.googlesource.com/543815
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481566}
[modify] https://crrev.com/cbbf1c73bae64dc528a052c38830b6e6956db048/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
[modify] https://crrev.com/cbbf1c73bae64dc528a052c38830b6e6956db048/components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.cc

Status: Fixed (was: Started)
Tentatively marking as fixed.
Status: Started (was: Fixed)
Re-opening, we need to handle the LoadURLExternally case.
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 17 2017

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

commit 68b11fe6571f4eba0044ba4396668f1085cb2bf5
Author: Charles Harrison <csharrison@chromium.org>
Date: Mon Jul 17 21:36:01 2017

Plumb WebTriggeringEventInfo through LoadURLExternally

This code path is used for e.g. shift-clicks coming from subframes
(strangely). By plumbing it into LoadURLExternally / OpenURL, we can
use this information in the popup blocker.

This patch makes the additional refactor to split LoadURLExternally
into two, LoadURLExternally and DownloadURL.

Bug:  733330 
Change-Id: I94476ecbf3f55ef26664d7c6a294e84fce7bba32
Reviewed-on: https://chromium-review.googlesource.com/565681
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Nick Carter <nick@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487251}
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/chrome/browser/download/download_browsertest.cc
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
[add] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/chrome/test/data/subresource_filter/iframe_spoof_click_popup.html
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/chrome/test/data/subresource_filter/window_open_spoof_click.html
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/content/public/renderer/render_frame.h
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/content/renderer/render_frame_impl.h
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/content/shell/test_runner/web_frame_test_client.cc
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/content/shell/test_runner/web_frame_test_client.h
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/content/shell/test_runner/web_frame_test_proxy.h
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/third_party/WebKit/Source/core/editing/LinkSelectionTest.cpp
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/third_party/WebKit/Source/core/frame/LocalFrameClient.h
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/third_party/WebKit/Source/web/LocalFrameClientImpl.h
[modify] https://crrev.com/68b11fe6571f4eba0044ba4396668f1085cb2bf5/third_party/WebKit/public/web/WebFrameClient.h

Blocking: 756089
Status: Fixed (was: Started)
Sort of obsolete since we moved the code to the popup blocker, but marking as fixed anyway since the work is completed.

Sign in to add a comment