Allow content script insertion on about:-URLs.

chrome/renderer/extensions/user_script_slave.cc

Index: chrome/renderer/extensions/user_script_slave.cc
diff --git a/chrome/renderer/extensions/user_script_slave.cc b/chrome/renderer/extensions/user_script_slave.cc
index bdaff10bb86692bbe69c3f3e69cadc438f90067c..46c586ce25cc8281acf27a72a3abd9e0d8f52a12 100644
--- a/chrome/renderer/extensions/user_script_slave.cc
+++ b/chrome/renderer/extensions/user_script_slave.cc
@@ -38,6 +38,7 @@
 #include "url/gurl.h"
 
 using blink::WebFrame;
+using blink::WebDocument;
 using blink::WebSecurityOrigin;
 using blink::WebSecurityPolicy;
 using blink::WebString;
@@ -194,6 +195,34 @@ GURL UserScriptSlave::GetDataSourceURLForFrame(const WebFrame* frame) {
   return GURL(data_source->request().url());
 }
 
+GURL UserScriptSlave::GetEffectiveDocumentURL(const WebFrame* frame,
+                                              const GURL& document_url,
+                                              bool match_about_blank) {
+  // Common scenario. If |match_about_blank| is false (as is the case in most
+  // extensions), or if the frame is not an about:-page, just return
+  // |document_url| (supposedly the URL of the frame).
+  if (!match_about_blank || !document_url.SchemeIs(content::kAboutScheme))
+    return document_url;
+
+  // Non-sandboxed about:blank and about:srcdoc pages inherit their security
+  // origin from their parent frame/window. So, traverse the frame/window
+  // hierarchy to find the closest non-about:-page and return its URL.
+  const WebFrame* parent = frame;
+  do {
+    parent = parent->parent() ? parent->parent() : parent->opener();
+  } while (parent != NULL &&
+           GURL(parent->document().url()).SchemeIs(content::kAboutScheme));
+
+  if (parent) {
+    // Only return the parent URL if the frame can access it.
+    const WebDocument& parent_document = parent->document();
+    if (frame->document().securityOrigin().canAccess(
+            parent_document.securityOrigin()))

[dcheng, 2014/05/02 17:23:19]

I believe this check should be inside the loop. Otherwise, you could traverse
through an iframe sandbox in the loop and miss that fact.

I would also like to see unit testing for this function--obviously it's already
partially covered by the new content script tests, but it'd be good to have a
more focused test on this portion as well.

[not at google - send to devlin, 2014/05/02 17:43:49]

oh, good point.
ah unit testing on the renderer...

[robwu, 2014/05/02 20:17:04]

I've intentionally used do-while instead of while to allow script insertion in
sandboxed iframes. The purpose of this function is to find out whether to insert
a script in an about:-frame that's a child of some host. Obviously, script
insertion should be (and is) forbidden if the extension does not have any access
to the parent's effective origin.
When an about:-frame is sandboxed and the extension has permissions to access
the parent document, then there are no issues with allowing the extension to run
a content script in the sandbox. This is consistent with the existing
implementation (for non-about URLs), which also ignores the sandbox attribute of
a frame.

Without allowing sandboxed frames, adverts could trivially bypass AdBlock
extensions by setting the sandbox attribute, e.g. <iframe srcdoc="some advert
here" sandbox=""></iframe>.
Should I use IN_PROC_BROWSER_TEST_F in a file called
user_script_slave_browsertest.cc? If not, could you give an example of what you
want to see?

[not at google - send to devlin, 2014/05/02 20:20:10]

Such an ad network would be severely limiting its effectiveness though, since it
won't have access to its own cookies.

[robwu, 2014/05/02 20:45:53]

Actually, never mind this discussion. If an iframe is sandboxed, then its
securityOrigin is unique, meaning that canAccess() will always return false, see
http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.cpp?revis...
and
http://src.chromium.org/viewvc/blink/trunk/Source/platform/weborigin/Security...

A to-be-added unit test will verify this invariant. Could you comment on my
question about the unit test, below?

[robwu, 2014/05/07 21:52:55]

Done.

+      return parent_document.url();
+  }
+  return document_url;
+}
+
 void UserScriptSlave::InjectScripts(WebFrame* frame,
                                     UserScript::RunLocation location) {
   GURL data_source_url = GetDataSourceURLForFrame(frame);
@@ -224,12 +253,15 @@ void UserScriptSlave::InjectScripts(WebFrame* frame,
     if (!extension)
       continue;
 
+    const GURL& document_url = GetEffectiveDocumentURL(
+        frame, data_source_url, script->match_about_blank());
+
     // Content scripts are not tab-specific.
     const int kNoTabId = -1;
     // We don't have a process id in this context.
     const int kNoProcessId = -1;
     if (!PermissionsData::CanExecuteScriptOnPage(extension,
-                                                 data_source_url,
+                                                 document_url,
                                                  frame->top()->document().url(),
                                                  kNoTabId,
                                                  script,