Since this is blocking crbug.com/628942, I am carrying forward the severity and milestones. 

I'm not very familiar with the page load deferrer, but out of curiosity, wouldn't it be better to put this lower level in either WebFrame or v8?  Extensions are only one caller of script injections.

Well, to make things interesting, devtools is allowed to run script ...

Ha of course it is.  Fun.

So right now, extensions code uses WebFrame::callFunctionEvenIfScriptDisabled(), which traces back to this four-year-old CL: https://codereview.chromium.org/9865015.  What *should* they do?  I've found various methods on frames, but it's unclear which is the right one (or how to morph the current call into that version).

Fun sidenote: callFunctionEvenIfScriptDisabled() has the comment that it bypasses "canExecute()", but "canExecute()" doesn't seem to exist: https://chromium.googlesource.com/chromium/src/+/b09ef0010c97bf25de363a91bdd4b865f4751ad4/third_party/WebKit/public/web/WebFrame.h#323

In my mind, it would be ideal if extensions could use WebLocalFrame:: requestExecuteScriptAndReturnValue: I think that should Just Work in the case of task suspension. However, that does mean that any code that depends on it executing synchronously would have to change, since the code gets notified about completion via a callback instead.

Unfortunately, requestExecuteScriptAndReturnValue() takes a WebScriptSource, which is basically a string, and these calls currently use a v8::Function.

I suppose we'll have to make a version of SuspendableScriptExecutor that works with v8 functions.

Ideally, we should deprecate and remove the old version too... but it looks like there are at least a few places that directly depend on this happening synchronously (Mojo and PPAPI).

rdevlin.cronin: 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

rdevlin.cronin: Uh oh! This issue still open and hasn't been updated in the last 29 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

Daniel, is this still a vuln given 628942 is fixed. Also, can you please help to find an owner.

Yes, this is still an important fix to make: we've fixed the immediate problem, but the fact that the extensions system can do this makes it so malicious content can poke at a bunch of untested states. This has triggered a UXSS, a URL spoof, and I'm sure there will be other bugs as well.

There is another UXSS bug, which relies on the same technique to work in issue 646610. We should really prioritize this higher and fix it as soon as we can. It is a High severity bug and has been open for close to two months. We must do better!

blink folks: I can help out with some of the extensions work here, but I think we're blocked until 6 - 11 have some resolution.  Is there someone that can own that part of the work?

We commit ourselves to a 60 day deadline for fixing for high severity vulnerabilities, and have exceeded it here. If you're unable to look into this soon, could you please find another owner or remove yourself so that this gets back into the security triage queue?

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

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

commit a9bc8ccd2becc952e4cea2d8a053247a481dd483
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Thu Oct 06 15:51:17 2016

[Blink] Modify SuspendableScriptExecutor to take a v8::Function

Allow SuspendableScriptExecutor to take a v8::Function so that it can be
used from Extensions code.  Also wire up WebLocalFrame and ScriptContext
to have handles to use it.  Convert a couple of basic extensions call
sites as a POC.

BUG= 629431 

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

[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/extensions/renderer/module_system.cc
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/extensions/renderer/render_frame_observer_natives.cc
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/extensions/renderer/script_context.cc
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/extensions/renderer/script_context.h
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/third_party/WebKit/Source/web/SuspendableScriptExecutor.cpp
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/third_party/WebKit/Source/web/SuspendableScriptExecutor.h
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/third_party/WebKit/public/web/WebLocalFrame.h

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

commit a3b4600f89b09569050c3cb4cb922022029405a6
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Mon Oct 10 20:21:44 2016

[Extensions] Convert some callers of ScriptContext::CallFunction

ScriptContext::CallFunction is deprecated (see bug for more info).
Convert some callers to use ScriptContext::CallFunctionSafe.

BUG= 629431 

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

[modify] https://crrev.com/a3b4600f89b09569050c3cb4cb922022029405a6/chrome/renderer/extensions/cast_streaming_native_handler.cc
[modify] https://crrev.com/a3b4600f89b09569050c3cb4cb922022029405a6/extensions/renderer/gc_callback.cc
[modify] https://crrev.com/a3b4600f89b09569050c3cb4cb922022029405a6/extensions/renderer/wake_event_page.cc

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

commit 078b2e8c3809e52b823269c009e68f39541085f5
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Tue Oct 11 00:21:35 2016

Remove chrome.test.runWithNativesEnabled

BUG= 654511 
BUG= 629431 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[modify] https://crrev.com/078b2e8c3809e52b823269c009e68f39541085f5/extensions/common/api/test.json
[modify] https://crrev.com/078b2e8c3809e52b823269c009e68f39541085f5/extensions/renderer/resources/test_custom_bindings.js
[modify] https://crrev.com/078b2e8c3809e52b823269c009e68f39541085f5/extensions/renderer/v8_context_native_handler.cc
[modify] https://crrev.com/078b2e8c3809e52b823269c009e68f39541085f5/extensions/renderer/v8_context_native_handler.h
[modify] https://crrev.com/078b2e8c3809e52b823269c009e68f39541085f5/extensions/test/data/api_test_base_unittest.js
[modify] https://crrev.com/078b2e8c3809e52b823269c009e68f39541085f5/ui/file_manager/externs/chrome_test.js

Can we mark this as Fixed now?

Merge-Triage since we'll want to take this into M55 once it's baked.

Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

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

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

commit aac3066303f55f1fd63d7dc3e04dde4e15c09d8a
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Tue Oct 18 21:27:46 2016

[Extensions] Convert some callers of ScriptContext::CallFunction

ScriptContext::CallFunction is deprecated (see bug for more info).
Since ModuleSystem::CallModuleFunction calls into CallFunction, add
a safe version of CallModuleFunction and begin converting callers.

BUG= 629431 

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

[modify] https://crrev.com/aac3066303f55f1fd63d7dc3e04dde4e15c09d8a/extensions/renderer/extension_helper.cc
[modify] https://crrev.com/aac3066303f55f1fd63d7dc3e04dde4e15c09d8a/extensions/renderer/json_schema_unittest.cc
[modify] https://crrev.com/aac3066303f55f1fd63d7dc3e04dde4e15c09d8a/extensions/renderer/module_system.cc
[modify] https://crrev.com/aac3066303f55f1fd63d7dc3e04dde4e15c09d8a/extensions/renderer/module_system.h
[modify] https://crrev.com/aac3066303f55f1fd63d7dc3e04dde4e15c09d8a/extensions/renderer/utils_unittest.cc

@Sheriffbot, this isn't fixed yet.  There are a couple dozen call sites that just called into js synchronously in extensions code that need to be fixed; it's not a one-off.

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

commit 389e45df0fc5fcebbc651eea8daa72f3989914a6
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Thu Oct 20 00:39:39 2016

[Extensions] Convert some callers of ModuleSystem::CallModuleMethod

ModuleSystem::CallModuleMethod is deprecated (see bug for more info).
Convert some callers to use ModuleSystem::CallModuleMethodSafe.

BUG= 629431 

Review-Url: https://chromiumcodereview.appspot.com/2436893002
Cr-Commit-Position: refs/heads/master@{#426357}

[modify] https://crrev.com/389e45df0fc5fcebbc651eea8daa72f3989914a6/extensions/renderer/dispatcher.cc
[modify] https://crrev.com/389e45df0fc5fcebbc651eea8daa72f3989914a6/extensions/renderer/messaging_bindings.cc
[modify] https://crrev.com/389e45df0fc5fcebbc651eea8daa72f3989914a6/extensions/renderer/module_system.cc
[modify] https://crrev.com/389e45df0fc5fcebbc651eea8daa72f3989914a6/extensions/renderer/module_system.h

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

commit 7b26cf7d53e618e91308852f47f6a406f7f86fb5
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Thu Oct 20 21:06:55 2016

[Extensions] Convert some callers of ModuleSystem::CallModuleMethod

ModuleSystem::CallModuleMethod is deprecated (see bug for more info).
Convert some callers to use ModuleSystem::CallModuleMethodSafe.

BUG= 629431 

Review-Url: https://chromiumcodereview.appspot.com/2426393003
Cr-Commit-Position: refs/heads/master@{#426595}

[modify] https://crrev.com/7b26cf7d53e618e91308852f47f6a406f7f86fb5/chrome/renderer/extensions/app_bindings.cc
[modify] https://crrev.com/7b26cf7d53e618e91308852f47f6a406f7f86fb5/chrome/renderer/extensions/webstore_bindings.cc
[modify] https://crrev.com/7b26cf7d53e618e91308852f47f6a406f7f86fb5/extensions/renderer/display_source_custom_bindings.cc
[modify] https://crrev.com/7b26cf7d53e618e91308852f47f6a406f7f86fb5/extensions/renderer/script_context.cc

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

commit 12a457ebc796770d6da36efb0d3fb3797e1a9c47
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Tue Oct 25 20:39:58 2016

[Extensions] Remove unused version of ScriptContext::CallFunction()

ScriptContext::CallFunction() is deprecated. Remove an unused override
of the function.

BUG= 629431 

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

[modify] https://crrev.com/12a457ebc796770d6da36efb0d3fb3797e1a9c47/extensions/renderer/script_context.cc
[modify] https://crrev.com/12a457ebc796770d6da36efb0d3fb3797e1a9c47/extensions/renderer/script_context.h

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

commit a9bc8ccd2becc952e4cea2d8a053247a481dd483
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Thu Oct 06 15:51:17 2016

[Blink] Modify SuspendableScriptExecutor to take a v8::Function

Allow SuspendableScriptExecutor to take a v8::Function so that it can be
used from Extensions code.  Also wire up WebLocalFrame and ScriptContext
to have handles to use it.  Convert a couple of basic extensions call
sites as a POC.

BUG= 629431 

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

[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/extensions/renderer/module_system.cc
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/extensions/renderer/render_frame_observer_natives.cc
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/extensions/renderer/script_context.cc
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/extensions/renderer/script_context.h
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/third_party/WebKit/Source/web/SuspendableScriptExecutor.cpp
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/third_party/WebKit/Source/web/SuspendableScriptExecutor.h
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/a9bc8ccd2becc952e4cea2d8a053247a481dd483/third_party/WebKit/public/web/WebLocalFrame.h

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

commit 57d2116813649148c761075f528db81ec51265db
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Fri Nov 04 06:00:16 2016

[Extensions + Blink] Account for user gesture in v8 function calls

Determine whether or not a user gesture is being processed when using
the SuspendableScriptExecutor to execute a v8 function.

BUG= 629431 

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

[modify] https://crrev.com/57d2116813649148c761075f528db81ec51265db/extensions/renderer/user_gestures_native_handler.cc
[modify] https://crrev.com/57d2116813649148c761075f528db81ec51265db/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp
[modify] https://crrev.com/57d2116813649148c761075f528db81ec51265db/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h
[modify] https://crrev.com/57d2116813649148c761075f528db81ec51265db/third_party/WebKit/Source/web/SuspendableScriptExecutor.cpp
[modify] https://crrev.com/57d2116813649148c761075f528db81ec51265db/third_party/WebKit/Source/web/SuspendableScriptExecutor.h
[modify] https://crrev.com/57d2116813649148c761075f528db81ec51265db/third_party/WebKit/Source/web/tests/WebFrameTest.cpp

[Automated comment] removing mislabelled merge-merged-2840

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

commit 82f54699b5a977642331f5f66d07c91365dbe706
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Tue Nov 08 01:18:07 2016

[Extensions] Convert some callers of ScriptContext::CallFunction

ScriptContext::CallFunction is deprecated (see bug for more info).
Convert some callers to use ScriptContext::SafeCallFunction.

BUG= 629431 

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

[modify] https://crrev.com/82f54699b5a977642331f5f66d07c91365dbe706/extensions/renderer/dispatcher.cc
[modify] https://crrev.com/82f54699b5a977642331f5f66d07c91365dbe706/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc

rdevlin.cronin: Is this finished now? Or is there still more to do? Thanks!

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

commit 0d94256f8ece8ddbb5f3a5c01b31fa42942ef5e7
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Fri Dec 09 15:34:23 2016

[Extensions] Convert more callers of CallModuleMethod

ModuleSystem::CallModuleMethod is deprecated (see bug for more info).
Convert some callers to use ModuleSystem::CallModuleMethodSafe.
Some callers need the results from the JS call, so introduce a
callback to the safe versions of CallModuleMethod and CallFunction.

BUG= 629431 

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

[modify] https://crrev.com/0d94256f8ece8ddbb5f3a5c01b31fa42942ef5e7/extensions/renderer/api_test_base.cc
[modify] https://crrev.com/0d94256f8ece8ddbb5f3a5c01b31fa42942ef5e7/extensions/renderer/module_system.cc
[modify] https://crrev.com/0d94256f8ece8ddbb5f3a5c01b31fa42942ef5e7/extensions/renderer/module_system.h
[modify] https://crrev.com/0d94256f8ece8ddbb5f3a5c01b31fa42942ef5e7/extensions/renderer/script_context.cc
[modify] https://crrev.com/0d94256f8ece8ddbb5f3a5c01b31fa42942ef5e7/extensions/renderer/script_context.h
[modify] https://crrev.com/0d94256f8ece8ddbb5f3a5c01b31fa42942ef5e7/extensions/renderer/script_injection.cc
[modify] https://crrev.com/0d94256f8ece8ddbb5f3a5c01b31fa42942ef5e7/extensions/renderer/script_injection.h
[modify] https://crrev.com/0d94256f8ece8ddbb5f3a5c01b31fa42942ef5e7/extensions/renderer/script_injection_callback.cc
[modify] https://crrev.com/0d94256f8ece8ddbb5f3a5c01b31fa42942ef5e7/extensions/renderer/script_injection_callback.h

@raymes, sorry for the delay.  There's still a bit more to do here.  (It's a long process to transition all of the extension calls into JS into something else.)
One of them is in extension messaging (https://chromium.googlesource.com/chromium/src/+/8e744a465d70e2ab1bf7959a66c3c65bd3850092/extensions/renderer/messaging_bindings.cc#167).  This will require a bit of trickery, but *should* be able to be converted.

The other is in the ScriptContext override for gin::Runner::Call(), which requires a synchronous return result.  This is used outside extensions, and not in any code I'm familiar with.

Jochen, who would be a good owner for that conversion?

Friendly ping from Security Sheriff.  Jochen -- question #41 for you.

issue 628942 is blocked by this one, but is already marked as fixed.

There are still two places where CallModuleMethod are called:
extensions/renderer/messaging_bindings.cc and extensions/renderer/script_context.cc

jochen@, could you help migrate the final two callers?  Thanks!

I'll handle the conversion in messaging_bindings.cc (which is just used by extensions code).

See comment 41 for why the call in script_context.cc is less straightforward.

Friendly ping from the security sheriff - how is the conversion for extension messaging going?

Friendly ping. :) This is a High severity bug that is pretty old.

jochen@: Can you fine a right owner for this?

sorry for not answering earlier, I only now saw the question!

yzshen@ is a good owner for this, let's split out a separate bug

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

commit e118f4d92ff6564c75fea95c07864a483cc42f29
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Fri Jun 09 20:32:21 2017

[Extensions Bindings] Accept null callbacks while ignoring schema

For legacy reasons, we have the method ConvertArgumentsIgnorningSchema,
which still needs to take into account the callback method as the final
parameter. Accept a null callback for these cases, and unittests for the
same. This fixes some of the context menus browser tests with native
bindings.

Also update the context menus handler bindings.

BUG= 629431 

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

[modify] https://crrev.com/e118f4d92ff6564c75fea95c07864a483cc42f29/extensions/renderer/api_signature.cc
[modify] https://crrev.com/e118f4d92ff6564c75fea95c07864a483cc42f29/extensions/renderer/api_signature_unittest.cc
[modify] https://crrev.com/e118f4d92ff6564c75fea95c07864a483cc42f29/extensions/renderer/resources/context_menus_handlers.js

D'oh, bug link mix-up.  Ignore #55 - it isn't relevant to this bug.

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

commit 978464a97d85014ca4414366789702be3b35fa15
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Sat Jun 10 01:30:04 2017

[Extensions Bindings] Request JS execution from messaging bindings

Instead of synchronously forcing JS execution in messaging bindings, use
safer alternatives to request JS execution. Since this modifies the
ability of the called function to return a result (synchronously, at
least), change messaging bindings to query for listeners natively rather
than in JS (which is also less likely to be incorrect).

This involves introducing a way to query if a given context has a
listener for an event. Provide this functionality for both native and
JS bindings. For native bindings, simply expose a method on
APIEventHandler. For JS bindings, make the NullAttachmentStrategy
register unmanaged events with the event bindings.

Also expose a way to get the Dispatcher statically from the messaging
bindings in order to get at the ExtensionBindingsSystem, and restructure
ownership of the dispatcher in extensions/shell.

BUG= 629431 

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

[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/chrome/renderer/extensions/chrome_extensions_renderer_client.cc
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/chrome/renderer/extensions/chrome_extensions_renderer_client.h
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/api_event_handler.cc
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/api_event_handler.h
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/dispatcher.h
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/event_bindings.cc
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/event_bindings.h
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/event_emitter.h
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/extension_bindings_system.h
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/extensions_renderer_client.h
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/js_extension_bindings_system.cc
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/js_extension_bindings_system.h
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/messaging_bindings.cc
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/messaging_bindings.h
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/native_extension_bindings_system.cc
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/native_extension_bindings_system.h
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/native_extension_bindings_system_unittest.cc
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/resources/event.js
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/resources/messaging.js
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/test_extensions_renderer_client.cc
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/renderer/test_extensions_renderer_client.h
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/shell/renderer/shell_content_renderer_client.cc
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/shell/renderer/shell_content_renderer_client.h
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/shell/renderer/shell_extensions_renderer_client.cc
[modify] https://crrev.com/978464a97d85014ca4414366789702be3b35fa15/extensions/shell/renderer/shell_extensions_renderer_client.h

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

commit 3043b941f5715ac0ec7d94f2f14d5d9ac9d32b1c
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Mon Jun 12 21:52:49 2017

[Extensions] Use CallModuleMethodSafe() in ScriptContext

In ScriptContext::OnResponseReceived(), instead of immediately executing
JS, request execution. This means we don't receive a result from the
execution. According to comments,

// In debug, the js will validate the callback parameters and return a
// string if a validation error has occured.

This appears to have been true at some point [1], but some time in the
last seven years, has been updated to no longer be the case. As such,
just remove any reference to the result value.

[1] https://chromium.googlesource.com/chromium/src/+/5c21a2eeaa92326a6a771e27c70b11b1ea52441a/chrome/renderer/resources/extension_process_bindings.js#114

BUG= 629431 

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

[modify] https://crrev.com/3043b941f5715ac0ec7d94f2f14d5d9ac9d32b1c/extensions/renderer/script_context.cc

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

commit cf5482bdd348723049d1024d1e95692c51739cc2
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Wed Jun 14 13:18:18 2017

[Extensions] Remove unsafe variants of CallModuleMethod

ModuleSystem::CallModuleMethod() didn't allow for asynchronous
execution. This method is no longer used, so remove it entirely.
Also move ScriptContext::CallFunction to be private, since it is
now only used from within that class.

BUG= 629431 

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

[modify] https://crrev.com/cf5482bdd348723049d1024d1e95692c51739cc2/extensions/renderer/module_system.cc
[modify] https://crrev.com/cf5482bdd348723049d1024d1e95692c51739cc2/extensions/renderer/module_system.h
[modify] https://crrev.com/cf5482bdd348723049d1024d1e95692c51739cc2/extensions/renderer/script_context.cc
[modify] https://crrev.com/cf5482bdd348723049d1024d1e95692c51739cc2/extensions/renderer/script_context.h

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

commit fedbe848f3024dd690f93545a337a2a6fb2aa81f
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Sat Jun 24 02:12:03 2017

[Extensions] Don't require() a module for calling a method

The ModuleSystem has an API to call a method of a given module, which
many callers use as a way to call into JS.  We've updated the JS call
itself to accept asynchronous JS execution, but if the module isn't
already active, the Require() call can result in JS execution.

Modify CallModuleMethodSafe() so that it won't Require() a module that
doesn't exist.  I think this should be okay, since I can't find any
cases of calling into a module where we'd still want to if the module
wasn't required.  The main time we would attempt to call into a module
that hasn't been required is to dispatch an event in JS bindings, but
if the Event module has never been instantiated, it's impossible that
any events have been registered.

Gracefully handle the case of the module not being required, and just
don't dispatch the method.

BUG= 629431 

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

[modify] https://crrev.com/fedbe848f3024dd690f93545a337a2a6fb2aa81f/extensions/renderer/api_test_base.cc
[modify] https://crrev.com/fedbe848f3024dd690f93545a337a2a6fb2aa81f/extensions/renderer/json_schema_unittest.cc
[modify] https://crrev.com/fedbe848f3024dd690f93545a337a2a6fb2aa81f/extensions/renderer/module_system.cc
[modify] https://crrev.com/fedbe848f3024dd690f93545a337a2a6fb2aa81f/extensions/renderer/module_system.h
[modify] https://crrev.com/fedbe848f3024dd690f93545a337a2a6fb2aa81f/extensions/renderer/utils_unittest.cc

So ... is this fixed after all these recent patches?

AFAIK, this is fixed except for the blocking bug (718047).

Per #c62: any reason to not mark this one as fixed?

Is this Fixed?

@66, 67: See https://bugs.chromium.org/p/chromium/issues/detail?id=629431#c63

I don't know common practices for security bugs.  If it's fine to close this out with an open blocker, then we can, but we'd need to make sure that we don't open this up too early.

I believe that it will be auto-disclosed in 14 weeks after being marked as Fixed.

I pinged  Issue 718047 , which might be Fixed now.

The blocker is now marked as Fixed, so can this be marked as Fixed now as well?

Should be!

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