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

Issue 655902 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Security



Sign in to add a comment

User-created BeforeInstallPromptEvent crashes when preventDefault() called

Project Member Reported by mgiuca@chromium.org, Oct 14 2016

Issue description

Version: 56
OS: Android

What steps will reproduce the problem?
(1) Set up a PWA site with a service worker, manifest, etc.
(2) Use attached index.js.
(3) Visit site.

The code that runs during load is:

  var e = new BeforeInstallPromptEvent({});
  e.preventDefault();
  window.setTimeout(() => {
    e.prompt()
        .then(() => console.log('prompt() succeeded'))
        .catch(e => console.error('prompt() failed: ' + e));
  }, 1000);

What is the expected output?
preventDefault has no effect.
After 1 second, console error: "prompt() failed: InvalidStateError ..."

What do you see instead?
Renderer crashes on the call to preventDefault.
10-14 13:52:58.130 11123 11241 F chromium: [FATAL:embedded_worker_registry.cc(260)] Check failed: base::ContainsKey(process_sender_map_, process_id). 

Note: I put in the call to prompt() to illustrate a potential problem after the crash is fixed: that the site is able to trigger a prompt without having received a legitimate beforeinstallprompt event from the UA. This should be prevented, regardless of whether preventDefault has been called on the event.
 
index.js
365 bytes View Download

Comment 1 by mmoroz@chromium.org, Oct 14 2016

Labels: Security_Severity-Medium Restrict-View-SecurityTeam
i'm setting Medium security severity, since this is a DCHECK failed. Please adjust it (to High or to Low) as needed.
Project Member

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

Labels: M-54
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 28 2016

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

commit 09f7b579849cc8725455d0468b03a0237a6b624d
Author: dominickn <dominickn@chromium.org>
Date: Fri Oct 28 01:44:15 2016

Convert app banners to use Mojo.

This CL converts the app banner system from Chrome IPC to Mojo and moves
all banner code from chrome/renderer to Blink. It also substantially
improves the layout test coverage for the BeforeInstallPromptEvent,
fixing various renderer crash bugs that are covered by the tests.

The key change is that the browser-side AppBannerManager makes a Mojo
request to Blink, which is intercepted by the WebLocalFrame and rerouted
to the AppBannerController. The AppBannerManager passes a bound
InterfaceRequest/Ptr in the Mojo request, which are used to create a
bound BeforeInstallPromptEvent. This ensures that when a
BeforeInstallPromptEvent is created, it already has a browser connection,
and does not need another asynchronous (and possibly racey) call to
establish full two-way communication with the browser process.

Several files in Blink's modules/app_banner directories that were solely
required for the AppBannerClient layer in chrome/renderer are deleted.
The existing layout tests are simplified by removing request_ids and
eliminating the AppBannerClient from ContentRendererClients. This
requires a dependency on content/public/common from
components/test_runner to allow a shim mojom::AppBannerService to be
injected.

dominickn@chromium.org is added to OWNERS for the Blink-side app
banner code.

BUG= 499704 , 655877 , 655902 

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

[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/DEPS
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/android/banners/app_banner_infobar_delegate_android.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/android/banners/app_banner_manager_android.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/android/banners/app_banner_manager_android.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/banners/app_banner_infobar_delegate_desktop.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/banners/app_banner_manager.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/banners/app_banner_manager.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/banners/app_banner_manager_desktop.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/banners/app_banner_manager_desktop.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/common/render_messages.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/renderer/BUILD.gn
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/chrome/renderer/banners/OWNERS
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/chrome/renderer/banners/app_banner_client.cc
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/chrome/renderer/banners/app_banner_client.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/renderer/chrome_content_renderer_client.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/renderer/chrome_render_frame_observer.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/renderer/chrome_render_frame_observer.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/BUILD.gn
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/DEPS
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/components/test_runner/app_banner_client.cc
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/components/test_runner/app_banner_client.h
[add] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/app_banner_service.cc
[add] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/app_banner_service.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/test_interfaces.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/test_interfaces.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/test_runner.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/test_runner.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/test_runner_for_specific_view.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/test_runner_for_specific_view.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/web_test_delegate.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/web_test_interfaces.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/web_test_interfaces.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/public/renderer/content_renderer_client.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/public/renderer/content_renderer_client.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/renderer/render_frame_impl.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/shell/renderer/layout_test/blink_test_runner.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/shell/renderer/layout_test/blink_test_runner.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/shell/renderer/layout_test/layout_test_content_renderer_client.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/LayoutTests/app_banner/app-banner-event-dispatch.html
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/LayoutTests/app_banner/app-banner-event-prompt.html
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/LayoutTests/app_banner/before-install-prompt-event-constructor.html
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/LayoutTests/app_banner/testrunner-resolve-crash.html
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/third_party/WebKit/Source/modules/app_banner/AppBannerCallbacks.cpp
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/third_party/WebKit/Source/modules/app_banner/AppBannerCallbacks.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/AppBannerController.cpp
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/AppBannerController.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/AppBannerPromptResult.cpp
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/AppBannerPromptResult.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/BUILD.gn
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/BeforeInstallPromptEvent.cpp
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/BeforeInstallPromptEvent.h
[add] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/DEPS
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/OWNERS
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/public/platform/modules/app_banner/OWNERS
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/third_party/WebKit/public/platform/modules/app_banner/WebAppBannerClient.h
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/third_party/WebKit/public/platform/modules/app_banner/WebAppBannerPromptReply.h
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/third_party/WebKit/public/platform/modules/app_banner/WebAppBannerPromptResult.h
[add] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/public/platform/modules/app_banner/app_banner.mojom
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/public/web/WebFrameClient.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/public/web/WebLocalFrame.h

Project Member

Comment 4 by sheriffbot@chromium.org, Oct 28 2016

dominickn: 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
Re-test following mojo conversion?
Project Member

Comment 6 by sheriffbot@chromium.org, Nov 11 2016

dominickn: Uh oh! This issue still open and hasn't been updated in the last 28 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
Status: Fixed (was: Assigned)
Project Member

Comment 8 by sheriffbot@chromium.org, Nov 15 2016

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

Comment 9 by sheriffbot@chromium.org, Feb 21 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

Sign in to add a comment