IsolatedWorldCSP: Use the isolated world CSP for font-src checks.

Instead of merely bypassing the CSP in the isolated world, enforce the
isolated world CSP. Also, plumb the correct DOMWrapperWorld for
ResourceLoaderOptions in PreloadHelper::PreloadIfNeeded.

BUG=1099975

Change-Id: I00ca5376c21692a547d2665533e4237e8609a843
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2276649
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791040}

third_party/blink/renderer/core/css/css_font_face_src_value.cc

@@ -83,7 +83,7 @@ bool CSSFontFaceSrcValue::HasFailedOrCanceledSubresources() const {
 
 FontResource& CSSFontFaceSrcValue::Fetch(ExecutionContext* context,
                                          FontResourceClient* client) const {
-  if (!fetched_) {
+  if (!fetched_ || fetched_->GetResource()->Options().world != world_) {
     ResourceRequest resource_request(absolute_resource_);
     resource_request.SetReferrerPolicy(
         ReferrerPolicyResolveDefault(referrer_.referrer_policy));
@@ -91,6 +91,7 @@ FontResource& CSSFontFaceSrcValue::Fetch(ExecutionContext* context,
     if (is_ad_related_)
       resource_request.SetIsAdResource();
     ResourceLoaderOptions options;
+    options.world = world_;
     options.initiator_info.name = fetch_initiator_type_names::kCSS;
     options.initiator_info.referrer = referrer_.referrer;
     FetchParameters params(std::move(resource_request), options);
@@ -98,7 +99,6 @@ FontResource& CSSFontFaceSrcValue::Fetch(ExecutionContext* context,
             features::kWebFontsCacheAwareTimeoutAdaption)) {
       params.SetCacheAwareLoadingEnabled(kIsCacheAwareLoadingEnabled);
     }
-    params.SetContentSecurityCheck(should_check_content_security_policy_);
     params.SetFromOriginDirtyStyleSheet(origin_clean_ != OriginClean::kTrue);
     const SecurityOrigin* security_origin = context->GetSecurityOrigin();
 
@@ -136,8 +136,6 @@ void CSSFontFaceSrcValue::RestoreCachedResourceIfNeeded(
   DCHECK(context->Fetcher());
 
   const KURL url = context->CompleteURL(absolute_resource_);
-  DCHECK_EQ(should_check_content_security_policy_,
-            fetched_->GetResource()->Options().content_security_policy_option);
   context->Fetcher()->EmulateLoadStartedForInspector(
       fetched_->GetResource(), url, mojom::RequestContextType::FONT,
       network::mojom::RequestDestination::kFont,

third_party/blink/renderer/core/css/css_font_face_src_value.h

@@ -26,10 +26,13 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_CSS_CSS_FONT_FACE_SRC_VALUE_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_CSS_FONT_FACE_SRC_VALUE_H_
 
+#include <utility>
+
 #include "base/memory/scoped_refptr.h"
 #include "third_party/blink/renderer/core/css/css_origin_clean.h"
 #include "third_party/blink/renderer/core/css/css_value.h"
 #include "third_party/blink/renderer/core/loader/resource/font_resource.h"
+#include "third_party/blink/renderer/platform/bindings/dom_wrapper_world.h"
 #include "third_party/blink/renderer/platform/weborigin/referrer.h"
 #include "third_party/blink/renderer/platform/wtf/casting.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
@@ -40,42 +43,39 @@ class ExecutionContext;
 
 class CORE_EXPORT CSSFontFaceSrcValue : public CSSValue {
  public:
-  static CSSFontFaceSrcValue* Create(
-      const String& specified_resource,
-      const String& absolute_resource,
-      const Referrer& referrer,
-      network::mojom::CSPDisposition should_check_content_security_policy,
-      OriginClean origin_clean,
-      bool is_ad_related) {
+  static CSSFontFaceSrcValue* Create(const String& specified_resource,
+                                     const String& absolute_resource,
+                                     const Referrer& referrer,
+                                     scoped_refptr<const DOMWrapperWorld> world,
+                                     OriginClean origin_clean,
+                                     bool is_ad_related) {
     return MakeGarbageCollected<CSSFontFaceSrcValue>(
         specified_resource, absolute_resource, referrer, false,
-        should_check_content_security_policy, origin_clean, is_ad_related);
+        std::move(world), origin_clean, is_ad_related);
   }
   static CSSFontFaceSrcValue* CreateLocal(
       const String& absolute_resource,
-      network::mojom::CSPDisposition should_check_content_security_policy,
+      scoped_refptr<const DOMWrapperWorld> world,
       OriginClean origin_clean,
       bool is_ad_related) {
     return MakeGarbageCollected<CSSFontFaceSrcValue>(
-        g_empty_string, absolute_resource, Referrer(), true,
-        should_check_content_security_policy, origin_clean, is_ad_related);
+        g_empty_string, absolute_resource, Referrer(), true, std::move(world),
+        origin_clean, is_ad_related);
   }
 
-  CSSFontFaceSrcValue(
-      const String& specified_resource,
-      const String& absolute_resource,
-      const Referrer& referrer,
-      bool local,
-      network::mojom::CSPDisposition should_check_content_security_policy,
-      OriginClean origin_clean,
-      bool is_ad_related)
+  CSSFontFaceSrcValue(const String& specified_resource,
+                      const String& absolute_resource,
+                      const Referrer& referrer,
+                      bool local,
+                      scoped_refptr<const DOMWrapperWorld> world,
+                      OriginClean origin_clean,
+                      bool is_ad_related)
       : CSSValue(kFontFaceSrcClass),
         absolute_resource_(absolute_resource),
         specified_resource_(specified_resource),
         referrer_(referrer),
         is_local_(local),
-        should_check_content_security_policy_(
-            should_check_content_security_policy),
+        world_(std::move(world)),
         origin_clean_(origin_clean),
         is_ad_related_(is_ad_related) {}
 
@@ -108,7 +108,7 @@ class CORE_EXPORT CSSFontFaceSrcValue : public CSSValue {
   String format_;
   const Referrer referrer_;
   const bool is_local_;
-  const network::mojom::CSPDisposition should_check_content_security_policy_;
+  const scoped_refptr<const DOMWrapperWorld> world_;
   const OriginClean origin_clean_;
   bool is_ad_related_;
 

third_party/blink/renderer/core/css/font_face_cache_test.cc

@@ -64,8 +64,8 @@ void FontFaceCacheTest::AppendTestFaceForCapabilities(const CSSValue& stretch,
   CSSFontFamilyValue* family_name =
       CSSFontFamilyValue::Create(kFontNameForTesting);
   CSSFontFaceSrcValue* src = CSSFontFaceSrcValue::CreateLocal(
-      kFontNameForTesting, network::mojom::CSPDisposition::DO_NOT_CHECK,
-      OriginClean::kTrue, false /* is_ad_related */);
+      kFontNameForTesting, nullptr /* world */, OriginClean::kTrue,
+      false /* is_ad_related */);
   CSSValueList* src_value_list = CSSValueList::CreateCommaSeparated();
   src_value_list->Append(*src);
   CSSPropertyValue properties[] = {

third_party/blink/renderer/core/css/parser/at_rule_descriptor_parser.cc

@@ -79,7 +79,7 @@ CSSValue* ConsumeFontFaceSrcURI(CSSParserTokenRange& range,
     return nullptr;
   CSSFontFaceSrcValue* uri_value(CSSFontFaceSrcValue::Create(
       url, context.CompleteURL(url), context.GetReferrer(),
-      context.ShouldCheckContentSecurityPolicy(),
+      context.JavascriptWorld(),
       context.IsOriginClean() ? OriginClean::kTrue : OriginClean::kFalse,
       context.IsAdRelated()));
 
@@ -100,14 +100,12 @@ CSSValue* ConsumeFontFaceSrcURI(CSSParserTokenRange& range,
 CSSValue* ConsumeFontFaceSrcLocal(CSSParserTokenRange& range,
                                   const CSSParserContext& context) {
   CSSParserTokenRange args = css_parsing_utils::ConsumeFunction(range);
-  network::mojom::CSPDisposition should_check_content_security_policy =
-      context.ShouldCheckContentSecurityPolicy();
   if (args.Peek().GetType() == kStringToken) {
     const CSSParserToken& arg = args.ConsumeIncludingWhitespace();
     if (!args.AtEnd())
       return nullptr;
     return CSSFontFaceSrcValue::CreateLocal(
-        arg.Value().ToString(), should_check_content_security_policy,
+        arg.Value().ToString(), context.JavascriptWorld(),
         context.IsOriginClean() ? OriginClean::kTrue : OriginClean::kFalse,
         context.IsAdRelated());
   }
@@ -116,7 +114,7 @@ CSSValue* ConsumeFontFaceSrcLocal(CSSParserTokenRange& range,
     if (!args.AtEnd())
       return nullptr;
     return CSSFontFaceSrcValue::CreateLocal(
-        family_name, should_check_content_security_policy,
+        family_name, context.JavascriptWorld(),
         context.IsOriginClean() ? OriginClean::kTrue : OriginClean::kFalse,
         context.IsAdRelated());
   }

third_party/blink/renderer/core/css/parser/css_parser_context.cc

@@ -47,7 +47,7 @@ CSSParserContext::CSSParserContext(const CSSParserContext* other,
                        other->is_html_document_,
                        other->use_legacy_background_size_shorthand_behavior_,
                        other->secure_context_mode_,
-                       other->should_check_content_security_policy_,
+                       other->world_,
                        use_counter_document,
                        other->resource_fetch_restriction_) {
   is_ad_related_ = other->is_ad_related_;
@@ -71,7 +71,7 @@ CSSParserContext::CSSParserContext(
           other->is_html_document_,
           other->use_legacy_background_size_shorthand_behavior_,
           other->secure_context_mode_,
-          other->should_check_content_security_policy_,
+          other->world_,
           use_counter_document,
           other->resource_fetch_restriction_) {
   is_ad_related_ = other->is_ad_related_;
@@ -91,7 +91,7 @@ CSSParserContext::CSSParserContext(CSSParserMode mode,
                        false,
                        false,
                        secure_context_mode,
-                       network::mojom::CSPDisposition::DO_NOT_CHECK,
+                       nullptr,
                        use_counter_document,
                        ResourceFetchRestriction::kNone) {}
 
@@ -132,10 +132,9 @@ CSSParserContext::CSSParserContext(
           document.GetExecutionContext()
               ? document.GetExecutionContext()->GetSecureContextMode()
               : SecureContextMode::kInsecureContext,
-          ContentSecurityPolicy::ShouldBypassMainWorld(
-              document.GetExecutionContext())
-              ? network::mojom::CSPDisposition::DO_NOT_CHECK
-              : network::mojom::CSPDisposition::CHECK,
+          document.GetExecutionContext()
+              ? document.GetExecutionContext()->GetCurrentWorld()
+              : nullptr,
           &document,
           resource_fetch_restriction) {}
 
@@ -151,9 +150,7 @@ CSSParserContext::CSSParserContext(const ExecutionContext& context)
                        true,
                        false,
                        context.GetSecureContextMode(),
-                       ContentSecurityPolicy::ShouldBypassMainWorld(&context)
-                           ? network::mojom::CSPDisposition::DO_NOT_CHECK
-                           : network::mojom::CSPDisposition::CHECK,
+                       context.GetCurrentWorld(),
                        IsA<LocalDOMWindow>(&context)
                            ? To<LocalDOMWindow>(context).document()
                            : nullptr,
@@ -170,11 +167,11 @@ CSSParserContext::CSSParserContext(
     bool is_html_document,
     bool use_legacy_background_size_shorthand_behavior,
     SecureContextMode secure_context_mode,
-    network::mojom::CSPDisposition policy_disposition,
+    scoped_refptr<const DOMWrapperWorld> world,
     const Document* use_counter_document,
     enum ResourceFetchRestriction resource_fetch_restriction)
     : base_url_(base_url),
-      should_check_content_security_policy_(policy_disposition),
+      world_(std::move(world)),
       origin_clean_(origin_clean),
       mode_(mode),
       match_mode_(match_mode),

third_party/blink/renderer/core/css/parser/css_parser_context.h

@@ -5,12 +5,14 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_CSS_PARSER_CSS_PARSER_CONTEXT_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_PARSER_CSS_PARSER_CONTEXT_H_
 
+#include "base/memory/scoped_refptr.h"
 #include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/css/css_property_names.h"
 #include "third_party/blink/renderer/core/css/css_resource_fetch_restriction.h"
 #include "third_party/blink/renderer/core/css/parser/css_parser_mode.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/frame/web_feature_forward.h"
+#include "third_party/blink/renderer/platform/bindings/dom_wrapper_world.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
 #include "third_party/blink/renderer/platform/loader/fetch/resource_loader_options.h"
 #include "third_party/blink/renderer/platform/weborigin/kurl.h"
@@ -73,7 +75,7 @@ class CORE_EXPORT CSSParserContext final
                    bool is_html_document,
                    bool use_legacy_background_size_shorthand_behavior,
                    SecureContextMode,
-                   network::mojom::CSPDisposition,
+                   scoped_refptr<const DOMWrapperWorld> world,
                    const Document* use_counter_document,
                    ResourceFetchRestriction resource_fetch_restriction);
 
@@ -125,8 +127,8 @@ class CORE_EXPORT CSSParserContext final
   const Document* GetDocument() const;
   const ExecutionContext* GetExecutionContext() const;
 
-  network::mojom::CSPDisposition ShouldCheckContentSecurityPolicy() const {
-    return should_check_content_security_policy_;
+  const scoped_refptr<const DOMWrapperWorld>& JavascriptWorld() const {
+    return world_;
   }
 
   // TODO(ekaramad): We currently only report @keyframes violations. We need to
@@ -163,7 +165,7 @@ class CORE_EXPORT CSSParserContext final
 
   KURL base_url_;
 
-  network::mojom::CSPDisposition should_check_content_security_policy_;
+  scoped_refptr<const DOMWrapperWorld> world_;
 
   // If true, allows reading and modifying of the CSS rules.
   // https://drafts.csswg.org/cssom/#concept-css-style-sheet-origin-clean-flag

third_party/blink/renderer/core/loader/preload_helper.cc

@@ -317,6 +317,7 @@ Resource* PreloadHelper::PreloadIfNeeded(
       GetFetchImportanceAttributeValue(params.importance));
 
   ResourceLoaderOptions options;
+  options.world = document.GetExecutionContext()->GetCurrentWorld();
   options.initiator_info.name = fetch_initiator_type_names::kLink;
   options.parser_disposition = parser_disposition;
   FetchParameters link_fetch_params(std::move(resource_request), options);

third_party/blink/renderer/core/loader/resource/font_resource_test.cc

@@ -134,8 +134,7 @@ TEST_F(CacheAwareFontResourceTest, CacheAwareFontLoading) {
   CSSFontFaceSrcValue* src_value = CSSFontFaceSrcValue::Create(
       url.GetString(), url.GetString(),
       Referrer(document.Url(), document.GetReferrerPolicy()),
-      network::mojom::CSPDisposition::DO_NOT_CHECK, OriginClean::kTrue,
-      false /* is_ad_related */);
+      nullptr /* world */, OriginClean::kTrue, false /* is_ad_related */);
 
   // Route font requests in this test through CSSFontFaceSrcValue::Fetch
   // instead of calling FontResource::Fetch directly. CSSFontFaceSrcValue

third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc

@@ -1339,6 +1339,10 @@ static bool IsDownloadOrStreamRequest(const ResourceRequest& request) {
 
 Resource* ResourceFetcher::MatchPreload(const FetchParameters& params,
                                         ResourceType type) {
+  // TODO(crbug.com/1099975): PreloadKey should be modified to also take into
+  // account the DOMWrapperWorld corresponding to the resource. This is because
+  // we probably don't want to share preloaded resources across different
+  // DOMWrapperWorlds to ensure predicatable behavior for preloads.
   auto it = preloads_.find(PreloadKey(params.Url(), type));
   if (it == preloads_.end())
     return nullptr;

third_party/blink/web_tests/http/tests/security/isolatedWorld/bypass-main-world-csp-font-src-expected.txt

-ALERT: Bypass main world's CSP with font-face.
+ALERT: With lax isolated world CSP
+ALERT: With strict isolated world CSP
 This test ensures that scripts run in isolated worlds marked with their own Content Security Policy aren't affected by the page's font-src directive.

third_party/blink/web_tests/http/tests/security/isolatedWorld/resources/font-src-bypass.js

@@ -3,26 +3,46 @@ if (window.testRunner) {
     testRunner.waitUntilDone();
 }
 
-window.addEventListener("message", function(message) {
-    testRunner.notifyDone();
+window.addEventListener('message', function(message) {
+  testCounter++;
+  test();
 }, false);
 
+let testCounter = 1;
+
 // This is needed because isolated worlds are not reset between test runs and a
 // previous test's CSP may interfere with this test. See
 // https://crbug.com/415845.
 testRunner.setIsolatedWorldInfo(1, null, null);
 
-function test() {
-    function setFontFace(num) {
-        var style = document.createElement("style");
-        style.innerText = "@font-face { font-family: 'remote'; src: url(/resources/Ahem.ttf); }";
-        document.getElementById('body').appendChild(style);
-        window.postMessage("next", "*");
-    }
+function setFontFace(num) {
+  let style = document.createElement('style');
+  style.innerText =
+      `@font-face { font-family: 'remote'; src: url(/resources/Ahem.ttf?num=${
+          num}); }`;
+  document.getElementById('body').appendChild(style);
+  window.postMessage('next', '*');
+}
 
-    alert("Bypass main world's CSP with font-face.");
-    testRunner.setIsolatedWorldInfo(1, "chrome-extension://123", "font-src 'self'");
-    testRunner.evaluateScriptInIsolatedWorld(1, String(eval("setFontFace")) + "\nsetFontFace(1);");
+function test() {
+  switch (testCounter) {
+    case 1:
+      alert('With lax isolated world CSP');
+      testRunner.setIsolatedWorldInfo(
+          1, 'chrome-extension://123', 'font-src *');
+      testRunner.evaluateScriptInIsolatedWorld(
+          1, String(eval('setFontFace')) + '\nsetFontFace(1);');
+      break;
+    case 2:
+      alert('With strict isolated world CSP');
+      testRunner.setIsolatedWorldInfo(
+          1, 'chrome-extension://123', 'font-src \'none\'');
+      testRunner.evaluateScriptInIsolatedWorld(
+          1, String(eval('setFontFace')) + '\nsetFontFace(2);');
+      break;
+    case 3:
+      testRunner.notifyDone();
+  }
 }
 
 document.addEventListener('DOMContentLoaded', test);

third_party/blink/web_tests/virtual/isolated_world_csp/http/tests/security/isolatedWorld/bypass-main-world-csp-font-src-expected.txt

+ALERT: With lax isolated world CSP
+ALERT: With strict isolated world CSP
+CONSOLE ERROR: Refused to load the font 'http://127.0.0.1:8000/resources/Ahem.ttf?num=2' because it violates the following Content Security Policy directive: "font-src 'none'".
+
+This test ensures that scripts run in isolated worlds marked with their own Content Security Policy aren't affected by the page's font-src directive.