Skip to content

GitLab

T
tangled
Commit d46788e5 authored by Devlin Cronin's avatar Devlin Cronin Committed by Commit Bot
Browse files
Options

[Extensions] Modify GetEffectiveDocumentURL() for MatchOriginAsFallback 

Modify GetEffectiveDocumentURL() to take a MatchOriginAsFallbackBehavior
to indicate in which cases the origin of a frame should be used as a
fallback, in the case where the frame's URL is insufficient (such as
about: and data: frames). If MatchOriginAsFallbackBehavior::kAlways is
specified (as is the case when match_origin_as_fallback is specified as
true in the manifest for a content script), the frame tree is no longer
traversed.

This allows scripts to inject on opaque frames where the extension has
access to the frame's initiator, but the frame tree contains cross-
origin frames.

Update the tests for the same, and add new tests for matching a frame's
origin as a fallback with path-specific match patterns; note that these
are not fully supported yet.

Bug: 55084
Change-Id: I86ccf56c3b05e92851ecca3748d8d973a098522f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2343323Reviewed-by: default avatarKaran Bhatia <karandeepb@chromium.org>
Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799192}
parent 8afd43ee
Hide whitespace changes
Inline Side-by-side
Showing with 158 additions and 73 deletions
chrome/browser/extensions/content_script_apitest.cc
View file @ d46788e5
...@@ -1141,6 +1141,12 @@ class ContentScriptRelatedFrameTest : public ContentScriptApiTest { ...@@ -1141,6 +1141,12 @@ class ContentScriptRelatedFrameTest : public ContentScriptApiTest {
} }
const GURL& null_document_url() const { return null_document_url_; } const GURL& null_document_url() const { return null_document_url_; }
const GURL& data_url() const { return data_url_; } const GURL& data_url() const { return data_url_; }
const GURL& path_specific_allowed_url() const {
return path_specific_allowed_url_;
}
const GURL& path_specific_iframe_url() const {
return path_specific_iframe_url_;
}
private: private:
static constexpr char kMarkerSpanId[] = "content-script-marker"; static constexpr char kMarkerSpanId[] = "content-script-marker";
...@@ -1160,6 +1166,11 @@ class ContentScriptRelatedFrameTest : public ContentScriptApiTest { ...@@ -1160,6 +1166,11 @@ class ContentScriptRelatedFrameTest : public ContentScriptApiTest {
GURL null_document_url_; GURL null_document_url_;
// A simple data URL. // A simple data URL.
GURL data_url_; GURL data_url_;
// A URL that matches a path-specific match pattern.
GURL path_specific_allowed_url_;
// A URL that matches the domain of a path-specific match pattern - but not
// the path component - which also has an iframe in the DOM.
GURL path_specific_iframe_url_;
// The test directory used to load our extension. // The test directory used to load our extension.
TestExtensionDir test_extension_dir_; TestExtensionDir test_extension_dir_;
...@@ -1182,6 +1193,10 @@ void ContentScriptRelatedFrameTest::SetUpOnMainThread() { ...@@ -1182,6 +1193,10 @@ void ContentScriptRelatedFrameTest::SetUpOnMainThread() {
embedded_test_server()->GetURL("chromium.org", "/iframe.html"); embedded_test_server()->GetURL("chromium.org", "/iframe.html");
null_document_url_ = embedded_test_server()->GetURL( null_document_url_ = embedded_test_server()->GetURL(
"chromium.org", "/extensions/null_document.html"); "chromium.org", "/extensions/null_document.html");
path_specific_allowed_url_ =
embedded_test_server()->GetURL("path-test.example", "/simple.html");
path_specific_iframe_url_ =
embedded_test_server()->GetURL("path-test.example", "/iframe.html");
data_url_ = GURL("data:text/html,<html>Hi</html>"); data_url_ = GURL("data:text/html,<html>Hi</html>");
constexpr char kManifest[] = constexpr char kManifest[] =
...@@ -1196,12 +1211,20 @@ void ContentScriptRelatedFrameTest::SetUpOnMainThread() { ...@@ -1196,12 +1211,20 @@ void ContentScriptRelatedFrameTest::SetUpOnMainThread() {
"all_frames": true, "all_frames": true,
%s %s
"match_about_blank": true "match_about_blank": true
}, {
"matches": ["http://path-test.example/simple.html"],
"js": ["script.js"],
"run_at": "document_end",
"all_frames": true,
%s
"match_about_blank": true
}] }]
})"; })";
const char* extra_property = ""; const char* extra_property = "";
if (IncludeMatchOriginAsFallback()) if (IncludeMatchOriginAsFallback())
extra_property = R"("match_origin_as_fallback": true,)"; extra_property = R"("match_origin_as_fallback": true,)";
std::string manifest = base::StringPrintf(kManifest, extra_property); std::string manifest =
base::StringPrintf(kManifest, extra_property, extra_property);
test_extension_dir_.WriteManifest(manifest); test_extension_dir_.WriteManifest(manifest);
std::string script = base::StringPrintf( std::string script = base::StringPrintf(
...@@ -1360,6 +1383,36 @@ IN_PROC_BROWSER_TEST_F(ContentScriptRelatedFrameTest, ...@@ -1360,6 +1383,36 @@ IN_PROC_BROWSER_TEST_F(ContentScriptRelatedFrameTest,
// precursor origin, which caused a crash during the document writing. // precursor origin, which caused a crash during the document writing.
} }
// Test content script injection into iframes when the script has a
// path-specific pattern.
IN_PROC_BROWSER_TEST_F(ContentScriptRelatedFrameTest,
FrameInjectionWithPathSpecificMatchPattern) {
// Open a page to the page that's same-origin with the match pattern, but
// doesn't match.
content::WebContents* tab = NavigateTab(path_specific_iframe_url());
// Navigate the child frame to the URL that matches the path requirement.
NavigateIframe(tab->GetMainFrame(), "frames[0]", path_specific_allowed_url());
content::RenderFrameHost* child_frame =
content::ChildFrameAt(tab->GetMainFrame(), 0);
EXPECT_EQ(path_specific_allowed_url(), child_frame->GetLastCommittedURL());
// The script should have ran in the child frame (which matches the pattern),
// but not the parent frame (which doesn't match the path component).
EXPECT_TRUE(DidScriptRunInFrame(child_frame));
EXPECT_FALSE(DidScriptRunInFrame(tab->GetMainFrame()));
// Now, navigate the iframe to an about:blank URL.
NavigateIframe(tab->GetMainFrame(), "frames[0]", about_blank());
// Unlike match_origin_as_fallback, match_about_blank will attempt to climb
// the frame tree to find an ancestor with path. This results in finding the
// parent frame, which doesn't match the script's pattern, and so the script
// does not inject.
EXPECT_EQ(about_blank(), child_frame->GetLastCommittedURL());
EXPECT_FALSE(DidScriptRunInFrame(child_frame));
}
// TODO(devlin): Similar to the above test, exercise one with a frame that // TODO(devlin): Similar to the above test, exercise one with a frame that
// closes its own parent. This needs to use tabs.executeScript (for timing // closes its own parent. This needs to use tabs.executeScript (for timing
// reasons), but is close enough to a content script test to re-use the same // reasons), but is close enough to a content script test to re-use the same
...@@ -1512,14 +1565,46 @@ IN_PROC_BROWSER_TEST_F(ContentScriptDataURLTest, ...@@ -1512,14 +1565,46 @@ IN_PROC_BROWSER_TEST_F(ContentScriptDataURLTest,
ASSERT_TRUE(data_url_host); ASSERT_TRUE(data_url_host);
EXPECT_TRUE(data_url_host->GetParent()); EXPECT_TRUE(data_url_host->GetParent());
EXPECT_EQ(data_url(), data_url_host->GetLastCommittedURL()); EXPECT_EQ(data_url(), data_url_host->GetLastCommittedURL());
// TODO(devlin): This should *probably* be allowed to inject at this point, // The extension should be allowed to inject since it has access to the
// since the extension has access to the related frame. // related frame. https://crbug.com/1111028.
// See https://crbug.com/1111028. EXPECT_TRUE(DidScriptRunInFrame(data_url_host));
// EXPECT_TRUE(DidScriptRunInFrame(data_url_host));
EXPECT_FALSE(DidScriptRunInFrame(data_url_host));
} }
} }
// Test content script injection into iframes when the script has a
// path-specific pattern.
IN_PROC_BROWSER_TEST_F(ContentScriptDataURLTest,
FrameInjectionWithPathSpecificMatchPattern) {
// Open a page to the page that's same-origin with the match pattern, but
// doesn't match.
content::WebContents* tab = NavigateTab(path_specific_iframe_url());
// Navigate the child frame to the URL that matches the path requirement.
NavigateIframe(tab->GetMainFrame(), "frames[0]", path_specific_allowed_url());
content::RenderFrameHost* child_frame =
content::ChildFrameAt(tab->GetMainFrame(), 0);
EXPECT_EQ(path_specific_allowed_url(), child_frame->GetLastCommittedURL());
// The script should have ran in the child frame (which matches the pattern),
// but not the parent frame (which doesn't match the path component).
EXPECT_TRUE(DidScriptRunInFrame(child_frame));
EXPECT_FALSE(DidScriptRunInFrame(tab->GetMainFrame()));
// Now, navigate the iframe to a data URL.
NavigateIframe(tab->GetMainFrame(), "frames[0]", data_url());
// Subtle: When using match_origin_as_fallback, the script will inject if it
// is same-origin with the initiator origin. In this case, even though no
// frames match the path of the script's match pattern, it will still inject
// in the data: frame, since the match pattern is same origin with the
// initiator (|path_specific_iframe_url|).
EXPECT_EQ(data_url(), child_frame->GetLastCommittedURL());
// TODO(devlin): Enable this check when we adjust the matching logic for
// these frames.
// EXPECT_TRUE(DidScriptRunInFrame(child_frame));
EXPECT_FALSE(DidScriptRunInFrame(child_frame));
}
// Test fixture which sets a custom NTP Page. // Test fixture which sets a custom NTP Page.
// TODO(karandeepb): Similar logic to set up a custom NTP is used elsewhere as // TODO(karandeepb): Similar logic to set up a custom NTP is used elsewhere as
// well. Abstract this away into a reusable test fixture class. // well. Abstract this away into a reusable test fixture class.
......
extensions/renderer/script_context.cc
View file @ d46788e5
...@@ -33,65 +33,49 @@ namespace extensions { ...@@ -33,65 +33,49 @@ namespace extensions {
namespace { namespace {
enum EffectiveUrlFlags { GURL GetEffectiveDocumentURL(
// Allow frame traversal past inaccessible parent contexts, such as a parent blink::WebLocalFrame* frame,
// of a sandboxed frame. const GURL& document_url,
kAllowInaccessibleParents = 1 << 0, MatchOriginAsFallbackBehavior match_origin_as_fallback,
// Allow matching on about:-scheme frames. bool allow_inaccessible_parents) {
kAllowAboutFrames = 1 << 1, auto should_consider_origin = [document_url, match_origin_as_fallback]() {
// Allow matching on data:-scheme frames. switch (match_origin_as_fallback) {
kAllowDataFrames = 1 << 2, case MatchOriginAsFallbackBehavior::kNever:
}; return false;
case MatchOriginAsFallbackBehavior::kMatchForAboutSchemeAndClimbTree:
// TODO(devlin): Modify this to take a return document_url.SchemeIs(url::kAboutScheme);
// UserScript::MatchOriginAsFallbackBehavior, and handle it appropriately. case MatchOriginAsFallbackBehavior::kAlways:
GURL GetEffectiveDocumentURL(blink::WebLocalFrame* frame, // TODO(devlin): Add more schemes here - blob, filesystem, etc.
const GURL& document_url, return document_url.SchemeIs(url::kAboutScheme) ||
int flags) { document_url.SchemeIs(url::kDataScheme);
// Common scenario: The frame is not an about: or data: frame, or we don't }
// match the relevant type. Just return |document_url| (supposedly the URL
// of the frame). NOTREACHED();
bool match_about_frames = (flags & kAllowAboutFrames) != 0;
bool match_data_urls = (flags & kAllowDataFrames) != 0;
auto should_traverse_tree = [match_about_frames,
match_data_urls](const GURL& url) {
return (match_about_frames && url.SchemeIs(url::kAboutScheme)) ||
(match_data_urls && url.SchemeIs(url::kDataScheme));
}; };
if (!should_traverse_tree(document_url)) // If we don't need to consider the origin, we're done.
if (!should_consider_origin())
return document_url; return document_url;
// For about: frames, the "security origin" is that of the controlling frame. // Get the "security origin" for the frame. For about: frames, this is the
// e.g., an about:blank frame on https://example.com will have the security // origin of that of the controlling frame - e.g., an about:blank frame on
// origin of https://example.com. // https://example.com will have the security origin of https://example.com.
blink::WebSecurityOrigin web_frame_origin = frame->GetSecurityOrigin(); // Other frames, like data: frames, will have an opaque origin. For these,
// we can get the precursor origin.
const blink::WebSecurityOrigin web_frame_origin = frame->GetSecurityOrigin();
const url::Origin frame_origin = web_frame_origin; const url::Origin frame_origin = web_frame_origin;
// Check the origin of the frame, including whether it is an opaque origin
// (like about:blank) that has a non-opaque precursor.
// Unfortunately, we still have to traverse the frame tree, because match
// patterns are associated with paths as well, not just origins. For instance,
// if an extension wants to run on google.com/maps/* with match_about_frames
// true, then it should run on about:blank frames created by google.com/maps,
// but not about:blank frames created by google.com (which is what the
// precursor tuple origin would be).
const url::SchemeHostPort& tuple_or_precursor_tuple = const url::SchemeHostPort& tuple_or_precursor_tuple =
frame_origin.GetTupleOrPrecursorTupleIfOpaque(); frame_origin.GetTupleOrPrecursorTupleIfOpaque();
// There is no valid tuple origin (which can happen in the case of e.g. a // When there's no valid tuple (which can happen in the case of e.g. a
// browser-initiated navigation to an opaque URL). Bail. // browser-initiated navigation to an opaque URL), there's no origin to
// fallback to. Bail.
if (!tuple_or_precursor_tuple.IsValid()) if (!tuple_or_precursor_tuple.IsValid())
return document_url; return document_url;
const url::Origin origin_or_precursor_origin = const url::Origin origin_or_precursor_origin =
url::Origin::Create(tuple_or_precursor_tuple.GetURL()); url::Origin::Create(tuple_or_precursor_tuple.GetURL());
// Check if the frame can access its precursor. It may not be allowed to in
// the case of a sandboxed iframe. We might still match on this, but we'll
// still check that the precursor tuple doesn't change.
bool allow_inaccessible_parents = (flags & kAllowInaccessibleParents) != 0;
if (!allow_inaccessible_parents && if (!allow_inaccessible_parents &&
!web_frame_origin.CanAccess( !web_frame_origin.CanAccess(
blink::WebSecurityOrigin(origin_or_precursor_origin))) { blink::WebSecurityOrigin(origin_or_precursor_origin))) {
...@@ -99,9 +83,27 @@ GURL GetEffectiveDocumentURL(blink::WebLocalFrame* frame, ...@@ -99,9 +83,27 @@ GURL GetEffectiveDocumentURL(blink::WebLocalFrame* frame,
return document_url; return document_url;
} }
// Non-sandboxed about:-scheme frames inherit their security origin from // Looks like the initiator origin is an appropriate fallback!
// their parent frame/window. So, traverse the frame/window hierarchy to find
// the closest non-about:-page and return its URL. if (match_origin_as_fallback == MatchOriginAsFallbackBehavior::kAlways) {
// The easy case! We use the origin directly. We're done.
return origin_or_precursor_origin.GetURL();
}
DCHECK_EQ(MatchOriginAsFallbackBehavior::kMatchForAboutSchemeAndClimbTree,
match_origin_as_fallback);
// Unfortunately, in this case, we have to climb the frame tree. This is for
// match patterns that are associated with paths as well, not just origins.
// For instance, if an extension wants to run on google.com/maps/* with
// match_about_blank true, then it should run on about:-scheme frames created
// by google.com/maps, but not about:-scheme frames created by google.com
// (which is what the precursor tuple origin would be).
// Traverse the frame/window hierarchy to find the closest non-about:-page
// with the same origin as the precursor and return its URL.
// Note: This can return the incorrect result, e.g. if a parent frame
// navigates a grandchild frame.
blink::WebFrame* parent = frame; blink::WebFrame* parent = frame;
GURL parent_url; GURL parent_url;
blink::WebDocument parent_document; blink::WebDocument parent_document;
...@@ -122,7 +124,8 @@ GURL GetEffectiveDocumentURL(blink::WebLocalFrame* frame, ...@@ -122,7 +124,8 @@ GURL GetEffectiveDocumentURL(blink::WebLocalFrame* frame,
? parent->ToWebLocalFrame()->GetDocument() ? parent->ToWebLocalFrame()->GetDocument()
: blink::WebDocument(); : blink::WebDocument();
// We reached the end of the ancestral chain without finding a valid parent. // We reached the end of the ancestral chain without finding a valid parent,
// or found a remote web frame (in which case, it's a different origin).
// Bail and use the original URL. // Bail and use the original URL.
if (parent_document.IsNull()) if (parent_document.IsNull())
return document_url; return document_url;
...@@ -149,11 +152,13 @@ GURL GetEffectiveDocumentURL(blink::WebLocalFrame* frame, ...@@ -149,11 +152,13 @@ GURL GetEffectiveDocumentURL(blink::WebLocalFrame* frame,
// window.frames[0].frames[0].location.href = 'about:blank'; // window.frames[0].frames[0].location.href = 'about:blank';
// In that case, the precursor origin tuple origin of frame "b" would be // In that case, the precursor origin tuple origin of frame "b" would be
// example.com, but the parent tuple origin is a.com. // example.com, but the parent tuple origin is a.com.
// Note that usually, this would have bailed earlier with a remote frame,
// but it may not if we're at the process limit.
return document_url; return document_url;
} }
parent_url = GURL(parent_document.Url()); parent_url = GURL(parent_document.Url());
} while (should_traverse_tree(parent_url)); } while (parent_url.SchemeIs(url::kAboutScheme));
DCHECK(!parent_url.is_empty()); DCHECK(!parent_url.is_empty());
DCHECK(!parent_document.IsNull()); DCHECK(!parent_document.IsNull());
...@@ -458,10 +463,15 @@ GURL ScriptContext::GetEffectiveDocumentURLForContext( ...@@ -458,10 +463,15 @@ GURL ScriptContext::GetEffectiveDocumentURLForContext(
bool match_about_blank) { bool match_about_blank) {
// Note: Do not allow matching inaccessible parent frames here; frames like // Note: Do not allow matching inaccessible parent frames here; frames like
// sandboxed frames should not inherit the privilege of their parents. // sandboxed frames should not inherit the privilege of their parents.
int flags = 0; constexpr bool allow_inaccessible_parents = false;
if (match_about_blank) // TODO(devlin): Determine if this could use kAlways instead of
flags |= kAllowAboutFrames; // kMatchForAboutSchemeAndClimbTree.
return GetEffectiveDocumentURL(frame, document_url, flags); auto match_origin_as_fallback =
match_about_blank
? MatchOriginAsFallbackBehavior::kMatchForAboutSchemeAndClimbTree
: MatchOriginAsFallbackBehavior::kNever;
return GetEffectiveDocumentURL(frame, document_url, match_origin_as_fallback,
allow_inaccessible_parents);
} }
// static // static
...@@ -472,19 +482,9 @@ GURL ScriptContext::GetEffectiveDocumentURLForInjection( ...@@ -472,19 +482,9 @@ GURL ScriptContext::GetEffectiveDocumentURLForInjection(
// We explicitly allow inaccessible parents here. Extensions should still be // We explicitly allow inaccessible parents here. Extensions should still be
// able to inject into a sandboxed iframe if it has access to the embedding // able to inject into a sandboxed iframe if it has access to the embedding
// origin. // origin.
int flags = kAllowInaccessibleParents; constexpr bool allow_inaccessible_parents = true;
switch (match_origin_as_fallback) { return GetEffectiveDocumentURL(frame, document_url, match_origin_as_fallback,
case MatchOriginAsFallbackBehavior::kNever: allow_inaccessible_parents);
break;
case MatchOriginAsFallbackBehavior::kMatchForAboutSchemeAndClimbTree:
flags |= kAllowAboutFrames;
break;
case MatchOriginAsFallbackBehavior::kAlways:
flags |= kAllowAboutFrames | kAllowDataFrames;
break;
}
return GetEffectiveDocumentURL(frame, document_url, flags);
} }
// Grants a set of content capabilities to this context. // Grants a set of content capabilities to this context.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment