[Extensions] Don't allow extensions to inject scripts into extension pages

extensions/renderer/script_injection.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/script_injection.h"
 
 #include <map>
 
 #include "base/lazy_instance.h"
 #include "base/metrics/histogram.h"
@@ skipping 122 matching lines @@
     return INJECTION_FINISHED;  // We're done.
   }
 
   blink::WebLocalFrame* web_frame = render_frame_->GetWebFrame();
   switch (injector_->CanExecuteOnFrame(
       injection_host_.get(), web_frame, tab_id_)) {
     case PermissionsData::ACCESS_DENIED:
       NotifyWillNotInject(ScriptInjector::NOT_ALLOWED);
       return INJECTION_FINISHED;  // We're done.
     case PermissionsData::ACCESS_WITHHELD:
-      // Note: we don't consider ACCESS_WITHHELD for child frames because there
-      // is nowhere to surface a request for a child frame.
-      // TODO(devlin): We should ask for permission somehow. crbug.com/491402.
-      if (web_frame->parent()) {
-        NotifyWillNotInject(ScriptInjector::NOT_ALLOWED);
-        return INJECTION_FINISHED;
-      }
-
       SendInjectionMessage(true /* request permission */);

[not at google - send to devlin, 2015/09/11 22:46:39]

This change means that you're always going to be sending this IPC, which is
wasteful given you're not even using its result.

On the other hand, you *could* actually take this opportunity to move these
checks into the browser (ActiveScriptController) to make it use RenderFrameHosts
rather than the WebContents.

Unfortunately that would commit to having these IPCs flying around and we might
want to wait until we can optimise only sending 1 IPC per origin rather than per
frame. You will have a better sense than me for whether taking the simpler
always-send-IPC approach is acceptable.

[Devlin, 2015/09/11 22:55:02]

Nope, because the logic to choose allowed or not allowed for a case of having no
UI surface is handled in the injection host now.  It will just return
ACCESS_DENIED here, so won't send any IPC.
Doesn't that also break things like doc-start completely?

[not at google - send to devlin, 2015/09/11 23:09:49]

I see. I read these methods in reverse.
Only change for withheld, not for everything, otherwise yes it would break
document_start all of the time.

Basically you'd remove the "web_frame->parent()" checks here - which is only hit
in the WITHHELD case - and instead do the check on the browser inside the
SendInjectionMessage handler (which would mean dealing with RenderFrameHost
etc).

as I said you may want to think about avoiding a flood of IPCs on every page
load with iframes, though.

       return INJECTION_WAITING;  // Wait around for permission.
     case PermissionsData::ACCESS_ALLOWED:
       InjectionResult result = Inject(scripts_run_info);
       // If the injection is blocked, we need to set the manager so we can
       // notify it upon completion.
       if (result == INJECTION_BLOCKED)
         async_completion_callback_ = async_completion_callback;
       return result;
   }
 
@@ skipping 137 matching lines @@
 
 void ScriptInjection::InjectCss() {
   std::vector<std::string> css_sources =
       injector_->GetCssSources(run_location_);
   blink::WebLocalFrame* web_frame = render_frame_->GetWebFrame();
   for (const std::string& css : css_sources)
     web_frame->document().insertStyleSheet(blink::WebString::fromUTF8(css));
 }
 
 }  // namespace extensions