extension: Checks the security token of the V8 context at GetModuleSystem().

When GetModuleSystem(arg) is called, we're using |arg|'s creation context, and it may violate the access over cross domain.

Fix GetModuleSystem so it checks the security token between the given context and the current context.  If they differ, returns |undefined|.

BUG=504011

[Yuki, 2015-07-09 09:21:57]

yukishiino@chromium.org changed reviewers:
+ haraken@chromium.org, kalman@chromium.org

[Yuki, 2015-07-09 09:21:58]

Could you guys review this CL?
kalman@, could you review as an owner?

I'm not an owner or expert of GetModuleSystem() and extension systems, so my
understanding is quite shallow.  I made this CL just as a minimum fix of Issue
504011.

Since there have been several security issues in the extension APIs, I think
Extension team may need some action against this kind of issues.

[not at google - send to devlin, 2015-07-09 16:22:19]

lg, but can you add a regression test for it? It can go in
extension_bindings_apitest.cc, our place for miscellaneous tests like these.

It doesn't need to be as complicated as the bug repro, I think that may break
over time. Instead, I'd decompose chrome.test.runWithModuleSystem into 2
functions, runWithNativesEnabled, and getModuleSystem, the latter of which
simply maps onto GetModuleSystem.

E.g. the only existing code which uses it would be changed from:

chrome.test.runWithModuleSystem(function(moduleSystem) {
  ...
});

to:

chrome.test.runWithNativesEnabled(function() {
  var moduleSystem = chrome.test.getModuleSystem();
  ...
});

and then you can write your test over chrome.test.getModuleSystem.

[Yuki, 2015-07-10 05:48:39]

On 2015/07/09 16:22:19, kalman wrote:
> lg, but can you add a regression test for it? It can go in
> extension_bindings_apitest.cc, our place for miscellaneous tests like these.
> 
> It doesn't need to be as complicated as the bug repro, I think that may break
> over time. Instead, I'd decompose chrome.test.runWithModuleSystem into 2
> functions, runWithNativesEnabled, and getModuleSystem, the latter of which
> simply maps onto GetModuleSystem.
> 
> E.g. the only existing code which uses it would be changed from:
> 
> chrome.test.runWithModuleSystem(function(moduleSystem) {
>   ...
> });
> 
> to:
> 
> chrome.test.runWithNativesEnabled(function() {
>   var moduleSystem = chrome.test.getModuleSystem();
>   ...
> });
> 
> and then you can write your test over chrome.test.getModuleSystem.

I'm not familiar in this area, and I don't know where
chrome.test.runWithModuleSystem is defined, what runWithNativesEnabled is, etc. 
I think I'm not the right person.

Could you take over this CL, or add a unit test on your side?

I've made this CL just because the security issue has been left unfixed for a
while.

[jochen (gone - plz use gerrit), 2015-07-13 12:01:16]

jochen@chromium.org changed reviewers:
+ jochen@chromium.org

[jochen (gone - plz use gerrit), 2015-07-13 12:01:17]

https://codereview.chromium.org/1231803002/diff/40001/extensions/renderer/v8_...
File extensions/renderer/v8_context_native_handler.cc (right):

https://codereview.chromium.org/1231803002/diff/40001/extensions/renderer/v8_...
extensions/renderer/v8_context_native_handler.cc:55:
args.GetIsolate()->GetCurrentContext()->GetSecurityToken())
i'd rather not duplicate the security check logic, but use the one from
BindingSecurity