Respect the clipboardRead and clipboardWrite permissions in content scripts.

extensions/renderer/dispatcher.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "extensions/renderer/dispatcher.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/debug/alias.h"
@@ skipping 201 matching lines @@
 }
 
 bool Dispatcher::IsExtensionActive(const std::string& extension_id) const {
   bool is_active =
       active_extension_ids_.find(extension_id) != active_extension_ids_.end();
   if (is_active)
     CHECK(extensions_.Contains(extension_id));
   return is_active;
 }
 
-std::string Dispatcher::GetExtensionID(const WebFrame* frame, int world_id) {
+std::string Dispatcher::GetExtensionID(const WebFrame* frame,
+                                       int world_id,
+                                       bool use_effective_url) {
   if (world_id != 0) {
     // Isolated worlds (content script).
     return ScriptInjection::GetExtensionIdForIsolatedWorld(world_id);
   }
 
   // TODO(kalman): Delete this check.
   if (frame->document().securityOrigin().isUnique())
     return std::string();
 
   // Extension pages (chrome-extension:// URLs).
   GURL frame_url = ScriptContext::GetDataSourceURLForFrame(frame);
+  frame_url = ScriptContext::GetEffectiveDocumentURL(
+      frame, frame_url, use_effective_url);
   return extensions_.GetExtensionOrAppIDByURL(frame_url);
 }
 
 void Dispatcher::DidCreateScriptContext(
     WebFrame* frame,
     const v8::Handle<v8::Context>& v8_context,
     int extension_group,
     int world_id) {
 #if !defined(ENABLE_EXTENSIONS)
   return;
 #endif
 
-  std::string extension_id = GetExtensionID(frame, world_id);
+  std::string extension_id = GetExtensionID(frame, world_id, false);
 
   const Extension* extension = extensions_.GetByID(extension_id);

[Devlin, 2014/09/03 22:09:16]

This is unnecessarily complicated.  We should just make it GetExtensionFromWorld
(or some other name) and have it return the extension
(ExtensionSet::GetExtensionOrAppIDByURL already looks up the full extension
anyway).

[Marijn Kruisselbrink, 2014/09/03 23:54:07]

Done, although I'm not entirely sure if it really was unnecessarily? I'm not
sure under which conditions "despite a context being associated with an
extension, no extension actually gets found", but by using
GetExtensionOrAppByURL instead of GetExtensionOrAppIDByURL followed by looking
up the extension by ID, I've eliminated at least one code paths that could
before have resulted in this LOG(ERROR). (I could of course change
GetExtensionFromFrameAndWorld back to first get the ID, but that doesn't really
seem much of an improvement compared to just leaving the code as it is).
So I'm tempted to undo the renaming/changing to GetExtensionFromFrameAndWorld
and instead go back to GetExtensionID

[Devlin, 2014/09/04 19:15:10]

Good point, so I don't really feel too strongly either way.

[Marijn Kruisselbrink, 2014/09/04 23:41:11]

Actually I kind of like having this GetExtensionFromFrameAndWorld method; I now
slightly rewrote it to keep the old behavior by first looking up the extension
id.

   if (!extension && !extension_id.empty()) {
     // There are conditions where despite a context being associated with an
     // extension, no extension actually gets found.  Ignore "invalid" because
     // CSP blocks extension page loading by switching the extension ID to
     // "invalid". This isn't interesting.
     if (extension_id != "invalid") {
       LOG(ERROR) << "Extension \"" << extension_id << "\" not found";
       RenderThread::Get()->RecordAction(
           UserMetricsAction("ExtensionNotFound_ED"));
     }
 
     extension_id = "";
   }
 
+  extension_id = GetExtensionID(frame, world_id, true);
+  const Extension* effective_extension = extensions_.GetByID(extension_id);

[Devlin, 2014/09/03 22:09:16]

If I recall correctly, GetEffectiveDocumentURL() only changes from frame_url if
it's a non-sandboxed about:blank. If this is the case, then |extension| on line
252 is almost certainly NULL, right?  (No extension will have an associated url
of about:blank.)  So if that's the case, do we really need both
|effective_extension| and |extension| in the ScriptContext, or could we just
make |extension| the effective extension?  Would that break things elsewhere?

[Marijn Kruisselbrink, 2014/09/03 23:54:07]

Yes, the ideal end state is to get rid of extension and only have
effective_extension. But, quoting kalman:
"Just changing that will change everything (the "ideal" case as I put it in the
bug). But I'm scared, and we'd need to do an audit of everywhere we use [it].

The hackier/conservative fix would be to store 2 Extensions in ScriptContext,
one derived from GetDataSourceURLForFrame and the other from
GetEffectiveDocumentURL, then use the latter in a CanClipboardRead/Write
method:"

[Devlin, 2014/09/04 19:15:10]

Ah.  That's too bad.  Okay.

+
+  GURL frame_url = ScriptContext::GetDataSourceURLForFrame(frame);
   Feature::Context context_type =
       ClassifyJavaScriptContext(extension,
                                 extension_group,
-                                ScriptContext::GetDataSourceURLForFrame(frame),
+                                frame_url,
                                 frame->document().securityOrigin());
+  Feature::Context effective_context_type = ClassifyJavaScriptContext(
+      effective_extension,
+      extension_group,
+      ScriptContext::GetEffectiveDocumentURL(frame, frame_url, true),
+      frame->document().securityOrigin());
 
   ScriptContext* context =
-      delegate_->CreateScriptContext(v8_context, frame, extension, context_type)
-          .release();
+      delegate_->CreateScriptContext(v8_context,
+                                     frame,
+                                     extension,
+                                     context_type,
+                                     effective_extension,
+                                     effective_context_type).release();
   script_context_set_.Add(context);
 
   // Initialize origin permissions for content scripts, which can't be
   // initialized in |OnActivateExtension|.
   if (context_type == Feature::CONTENT_SCRIPT_CONTEXT)
     InitOriginPermissions(extension);
 
   {
     scoped_ptr<ModuleSystem> module_system(
         new ModuleSystem(context, &source_map_));
@@ skipping 1018 matching lines @@
     return v8::Handle<v8::Object>();
 
   if (bind_name)
     *bind_name = split.back();
 
   return bind_object.IsEmpty() ? AsObjectOrEmpty(GetOrCreateChrome(context))
                                : bind_object;
 }
 
 }  // namespace extensions