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

Issue 608641 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

consider moving the whitelisting checks for isolated worlds to ScriptController::canExecuteScript

Project Member Reported by jochen@chromium.org, May 3 2016

Issue description

This would require specifying at all call-sites whether in which world the script is going to be executed.

We can't just look at the current context to do make this decision, as a <script> element inserted by an isolated world still shouldn't get executed if js execution is disabled.

Also, so far the check is done before calling canExecuteScript, e.g.,

if (scriptState->isMainWorld() && !frame()->script().canExecuteScripts())

if we do this change proposed here, we should also change all those sites to use the new pattern.
 
This issue has been untriaged for a while. Is this something that is still relevant? canExecuteScript has now been moved to ExecutionContext, and I'm not entirely sure where these whitelisting checks are currently done.

Comment 2 by jochen@chromium.org, Apr 27 2017

Kentaro, you proposed this originally?
We need to check if how many call sites will need the isMainWorld check. If V8EventListener is the only case, the current code looks fine. Otherwise we can consider implementing ScriptState::canExecuteScripts(), which does the isMainWorld check and calls ExecutionContext::canExecuteScripts().

Owner: jochen@chromium.org
Status: Assigned (was: Untriaged)
It still looks like V8EventListener is the only call site that does the main world check, but I'm not sure if other call sites need this check.
Status: WontFix (was: Assigned)
Cc: yukiy@google.com peria@chromium.org
Owner: yukishiino@chromium.org
Status: Assigned (was: WontFix)
I want to re-open this bug.
We should not forbid script execution according to the execution setting in isolated world. It should be operated by disabling Chrome extension.
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 23

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

commit 856fc5567263cadd3edcfd56c1d8e6e6b6873f07
Author: Yuki Yamada <yukiy@google.com>
Date: Thu Aug 23 11:31:06 2018

Check script execution is forbidden or not only in MainWorld

We should not forbid script execution in isolated worlds when
dispatching events (it should be operated by disabling Chrome
extension), so checking execution setting is necessary only in main
world.
V8EventListenerOrEventHandler::CallListenerFunction() already
implements this, so this CL make
GeneratedCodeHelper::IsCallbackRunnable() to do the same conditional
check for other callbacks generated by IDL files.

Test for this is already exists:
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/fast/events/events-in-isolated-world.html

Bug:  872138 , 608286, 608641
Change-Id: Ibc38e6033d6d0462362012f1c49271548c26a8ec
Reviewed-on: https://chromium-review.googlesource.com/1186212
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Commit-Queue: Yuki Yamada <yukiy@google.com>
Cr-Commit-Position: refs/heads/master@{#585449}
[modify] https://crrev.com/856fc5567263cadd3edcfd56c1d8e6e6b6873f07/third_party/blink/renderer/bindings/core/v8/generated_code_helper.cc

Sign in to add a comment