Block content scripts from executing until user grants permission

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 <vector>
 
 #include "base/lazy_instance.h"
 #include "base/metrics/histogram.h"
 #include "content/public/common/url_constants.h"
+#include "content/public/renderer/render_view.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "extensions/renderer/dom_activity_logger.h"
 #include "extensions/renderer/extension_groups.h"
 #include "extensions/renderer/script_context.h"
 #include "extensions/renderer/user_script_slave.h"
 #include "grit/renderer_resources.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebFrame.h"
 #include "third_party/WebKit/public/web/WebScriptSource.h"
+#include "third_party/WebKit/public/web/WebView.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "url/gurl.h"
 
 namespace extensions {
 
 namespace {
 
+// The id of the next pending injection.
+int64 g_next_pending_id = 0;
+
 // These two strings are injected before and after the Greasemonkey API and
 // user script to wrap it in an anonymous scope.
 const char kUserScriptHead[] = "(function (unsafeWindow) {\n";
 const char kUserScriptTail[] = "\n})(window);";
 
 // Greasemonkey API source that is injected with the scripts.
 struct GreasemonkeyApiJsString {
   GreasemonkeyApiJsString();
   blink::WebScriptSource source;
 };
@@ skipping 10 matching lines @@
     LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
 ScriptInjection::ScriptsRunInfo::ScriptsRunInfo() : num_css(0u), num_js(0u) {
 }
 
 ScriptInjection::ScriptsRunInfo::~ScriptsRunInfo() {
 }
 
+struct ScriptInjection::PendingInjection {
+  PendingInjection(const blink::WebString& web_frame_name,
+                   UserScript::RunLocation run_location,
+                   int page_id);
+  ~PendingInjection();
+
+  // The globally-unique id of this request.
+  int64 id;

[not at google - send to devlin, 2014/05/21 15:01:07]

it seems like |page_id| is enough to distinguish requests? you're checking
script injection per extension, so if an extension is allowed then all its
script injections (and into the future, for that page) should be allowed too.

(note to future self/devlin: check for race conditions with active tab and/or
just generally optional permissions)

having this modelled as a request ID can't *hurt* I suppose but it is redundant
and complicates things a little. I think the only code that would need to change
is not terminating the loop in OnContentScriptGrantedPermission?

[Devlin, 2014/05/21 17:05:11]

We'd actually need page id + extension id, because just sending back a page id
doesn't let us know which extension is approved.  Just by that, the int64 is
smaller and faster to send across (and I think int64 is preferred to even an
extension id string for IPC, but the security folks would know much better).

[not at google - send to devlin, 2014/05/21 17:36:23]

good point.

+
+  // The name of the web frame into which to inject.
+  blink::WebString web_frame_name;

[not at google - send to devlin, 2014/05/21 15:01:07]

why not just hold onto the WebFrame pointer? code below doesn't imply it's
unsafe given you're observing NotifyFrameDetached.

[Devlin, 2014/05/21 17:05:11]

Good point.  Let's try that.

+
+  // The run location to inject at.
+  // Note: This could be a lie - we might inject well after this run location
+  // has come and gone. But we need to know it to know which scripts to inject.
+  UserScript::RunLocation run_location;
+
+  // The corresponding page id, to protect against races.
+  int page_id;
+};
+
+ScriptInjection::PendingInjection::PendingInjection(
+    const blink::WebString& web_frame_name,
+    UserScript::RunLocation run_location,
+    int page_id)
+    : id(g_next_pending_id++),
+      web_frame_name(web_frame_name),
+      run_location(run_location),
+      page_id(page_id) {
+}
+
+ScriptInjection::PendingInjection::~PendingInjection() {
+}
+
 // static
 GURL ScriptInjection::GetDocumentUrlForFrame(blink::WebFrame* frame) {
   GURL data_source_url = ScriptContext::GetDataSourceURLForFrame(frame);
   if (!data_source_url.is_empty() && frame->isViewSourceModeEnabled()) {
     data_source_url = GURL(content::kViewSourceScheme + std::string(":") +
                            data_source_url.spec());
   }
 
   return data_source_url;
 }
 
 ScriptInjection::ScriptInjection(
     scoped_ptr<UserScript> script,
     UserScriptSlave* user_script_slave)
     : script_(script.Pass()),
       extension_id_(script_->extension_id()),
       user_script_slave_(user_script_slave),
       is_standalone_or_emulate_greasemonkey_(
           script_->is_standalone() || script_->emulate_greasemonkey()) {
 }
 
 ScriptInjection::~ScriptInjection() {
 }
 
+void ScriptInjection::InjectIfAllowed(blink::WebFrame* frame,
+                                      UserScript::RunLocation run_location,
+                                      const GURL& document_url,
+                                      ScriptsRunInfo* scripts_run_info) {
+  if (!WantsToRun(frame, run_location, document_url))
+    return;
+
+  const Extension* extension = user_script_slave_->GetExtension(extension_id_);
+  DCHECK(extension);  // WantsToRun() should be false if there's no extension.

[not at google - send to devlin, 2014/05/21 15:01:07]

CHECK

[Devlin, 2014/05/21 17:05:11]

If you insist...

My thinking is generally that if you can *know* (logically) that something is a
certain way, there's no point in CHECKing (and slowing down a production build
by x fractions of a microsecond).  Instead, DCHECK, and if there's something
wrong, we'll learn about it that way.  But, anyway.  Just my $0.02.  Done.

[not at google - send to devlin, 2014/05/21 17:36:23]

I trust many things, but not the existence of extensions.

+
+  content::RenderView* top_render_view =
+      content::RenderView::FromWebView(frame->top()->view());

[not at google - send to devlin, 2014/05/21 15:01:07]

please write a nice comment explaining why you're checking the top render view
here, not the render view for |frame|.

[Devlin, 2014/05/21 17:05:11]

Done.

+  if (PermissionsData::RequiresActionForScriptExecution(extension)) {
+    int page_id = top_render_view->GetPageId();
+    ScopedVector<PendingInjection>::iterator pending_injection =
+        pending_injections_.insert(
+            pending_injections_.end(),
+            new PendingInjection(frame->uniqueName(), run_location, page_id));
+
+    top_render_view->Send(
+        new ExtensionHostMsg_RequestContentScriptPermission(
+            top_render_view->GetRoutingID(),
+            extension->id(),
+            page_id,
+            (*pending_injection)->id));
+  } else {
+    Inject(frame, run_location, scripts_run_info);
+  }
+}
+
+bool ScriptInjection::NotifyScriptPermitted(
+    int64 request_id,
+    content::RenderView* render_view,
+    ScriptsRunInfo* scripts_run_info,
+    blink::WebFrame** frame_out) {
+  if (!render_view)

[not at google - send to devlin, 2014/05/21 15:01:07]

does this actually get called with a null render view..?

[Devlin, 2014/05/21 17:05:11]

I'd certainly hope not, but I hate to make crashing assumptions. ;)  Want me to
change it to a CHECK(), omit it, or leave it as-is?

[not at google - send to devlin, 2014/05/21 17:36:23]

I'd take it out entirely. it's seemingly arbitrary to check the render view and
not, say, |scripts_run_info|. if there's a good reasons to check it (i.e. my
question) then do so but doesn't seem that way.

[Devlin, 2014/05/21 18:28:28]

Done.

+    return false;
+
+  ScopedVector<PendingInjection>::iterator iter = pending_injections_.begin();
+  while (iter != pending_injections_.end() && (*iter)->id != request_id)
+    ++iter;
+
+  // No matching request.
+  if (iter == pending_injections_.end())
+    return false;
+
+  // We found the request, so pull it out of the pending list.
+  scoped_ptr<PendingInjection> pending_injection(*iter);
+  pending_injections_.weak_erase(iter);
+
+  // Ensure the WebView, WebFrame, Extension, and Page ID all still exist and
+  // match. Otherwise, don't inject.
+  if (render_view->GetPageId() != pending_injection->page_id)
+    return false;
+
+  blink::WebView* web_view = render_view->GetWebView();
+  if (!web_view)
+    return false;
+
+  blink::WebFrame* web_frame =
+      web_view->findFrameByName(pending_injection->web_frame_name);
+  if (!web_frame)
+    return false;
+
+  const Extension* extension = user_script_slave_->GetExtension(extension_id_);
+  if (!extension)
+    return false;
+
+  // Everything matches! Inject the script.
+  if (frame_out)
+    *frame_out = web_frame;
+  Inject(web_frame, pending_injection->run_location, scripts_run_info);
+  return true;
+}
+
+void ScriptInjection::NotifyFrameDetached(blink::WebFrame* frame) {

[not at google - send to devlin, 2014/05/21 15:01:07]

ditto FrameDetached

[Devlin, 2014/05/21 17:05:11]

Done.

+  // Any pending injections associated with the given frame will never run.
+  // Remove them.
+  for (ScopedVector<PendingInjection>::iterator iter =
+           pending_injections_.begin();
+       iter != pending_injections_.end();) {
+    if ((*iter)->web_frame_name == frame->uniqueName())
+      pending_injections_.erase(iter);
+    else
+      ++iter;
+  }
+}
+
 bool ScriptInjection::WantsToRun(blink::WebFrame* frame,
                                  UserScript::RunLocation run_location,
                                  const GURL& document_url) const {
   if (frame->parent() && !script_->match_all_frames())
     return false;  // Only match subframes if the script declared it wanted to.
 
   const Extension* extension = user_script_slave_->GetExtension(extension_id_);
   // Since extension info is sent separately from user script info, they can
   // be out of sync. We just ignore this situation.
   if (!extension)
@@ skipping 99 matching lines @@
   scripts_run_info->num_css += css_scripts.size();
   for (UserScript::FileList::const_iterator iter = css_scripts.begin();
        iter != css_scripts.end();
        ++iter) {
     frame->document().insertStyleSheet(
         blink::WebString::fromUTF8(iter->GetContent().as_string()));
   }
 }
 
 }  // namespace extensions