[Extensions] Fix import of dynamic URL resources

Extensions have a minimum CSP applied to their isolated worlds. This
CSP restricts remotely-hosted code (by design) by limiting script
sources to `self`. However, this causes problems when trying to
`import()` resources that are exposed in web-accessible resources with
`use_dynamic_url` set to true, since those resources will be (initially)
served over the extension's dynamic origin.

Modify the extension's isolated world CSP to explicitly allow resources
from that extension's dynamic origin, and add a browsertest to exercise
the same.

Fixed: 363027634
Change-Id: I73af26ce15e9164b544e9ee70f7c94ade162d6ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5955629
Reviewed-by: Tim <tjudkins@chromium.org>
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1373148}

chrome/browser/extensions/content_security_policy_apitest.cc

@@ -187,4 +187,68 @@ IN_PROC_BROWSER_TEST_F(ExtensionCspApiTest,
   EXPECT_EQ(2u, console_observer.messages().size());
 }
 
+// A simple subclass that also sets up page navigation with the host resolver.
+class ExtensionCspApiTestWithPageNavigation : public ExtensionCspApiTest {
+ public:
+  ExtensionCspApiTestWithPageNavigation() = default;
+  ~ExtensionCspApiTestWithPageNavigation() override = default;
+
+  void SetUpOnMainThread() override {
+    ExtensionCspApiTest::SetUpOnMainThread();
+    host_resolver()->AddRule("*", "127.0.0.1");
+    ASSERT_TRUE(StartEmbeddedTestServer());
+  }
+};
+
+// Exercises importing resources exposed in web-accessible resources with
+// dynamic URLs in content scripts.
+// Regression test for https://crbug.com/363027634.
+IN_PROC_BROWSER_TEST_F(ExtensionCspApiTestWithPageNavigation,
+                       ContentScriptsCanImportDynamicUrlResources) {
+  static constexpr char kManifest[] =
+      R"({
+           "name": "Test Extension",
+           "manifest_version": 3,
+           "version": "0.1",
+           "web_accessible_resources": [{
+             "resources": ["accessible_resource.js"],
+             "matches": ["http://example.com/*"],
+             "use_dynamic_url": true
+           }],
+           "content_scripts": [{
+             "matches": ["http://example.com/*"],
+             "js": ["content_script.js"]
+           }]
+         })";
+  // The content script attempts to import() a resource that's exposed in
+  // web-accessible resources using the extension's dynamic URL. This resource
+  // then exposes a `passTest()` function, which will pass the test.
+  static constexpr char kContentScriptJs[] =
+      R"((async () => {
+           try {
+             const url = chrome.runtime.getURL('./accessible_resource.js');
+             const mod = await import(url);
+             mod.passTest();
+           } catch(e) {
+             chrome.test.notifyFail('Failed to import: ' + e.toString());
+           }
+         })();)";
+  static constexpr char kAccessibleResourceJs[] =
+      R"(export function passTest() {
+           chrome.test.notifyPass();
+         })";
+
+  TestExtensionDir test_dir;
+  test_dir.WriteManifest(kManifest);
+  test_dir.WriteFile(FILE_PATH_LITERAL("content_script.js"), kContentScriptJs);
+  test_dir.WriteFile(FILE_PATH_LITERAL("accessible_resource.js"),
+                     kAccessibleResourceJs);
+
+  const GURL url =
+      embedded_test_server()->GetURL("example.com", "/simple.html");
+  ASSERT_TRUE(RunExtensionTest(test_dir.UnpackedPath(),
+                               {.page_url = url.spec().c_str()}, {}))
+      << message_;
+}
+
 }  // namespace extensions

extensions/common/extension.cc

@@ -278,15 +278,17 @@ scoped_refptr<Extension> Extension::Create(const base::FilePath& path,
   scoped_refptr<Extension> extension = new Extension(path, std::move(manifest));
   extension->install_warnings_.swap(install_warnings);
 
+  // Some manifest parsing may require the dynamic URL to be present on the
+  // extension; instantiate it now.
+  extension->guid_ = base::Uuid::GenerateRandomV4();
+  extension->dynamic_url_ = Extension::GetBaseURLFromExtensionId(
+      extension->guid_.AsLowercaseString());
+
   if (!extension->InitFromValue(flags, &error)) {
     *utf8_error = base::UTF16ToUTF8(error);
     return nullptr;
   }
 
-  extension->guid_ = base::Uuid::GenerateRandomV4();
-  extension->dynamic_url_ = Extension::GetBaseURLFromExtensionId(
-      extension->guid_.AsLowercaseString());
-
   return extension;
 }
 

extensions/common/manifest_handlers/csp_info.cc

@@ -11,6 +11,7 @@
 #include "base/feature_list.h"
 #include "base/no_destructor.h"
 #include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "extensions/common/csp_validator.h"
@@ -45,11 +46,21 @@ static const char kDefaultMV3CSP[] = "script-src 'self';";
 static const char kMinimumMV3CSP[] =
     "script-src 'self' 'wasm-unsafe-eval' 'inline-speculation-rules'; "
     "object-src 'self';";
+// The minimum CSP to be used in isolated worlds. The placeholder is for the
+// extension's dynamic URL.
+constexpr char kMinimumMV3IsolatedWorldCSPTemplate[] =
+    "script-src 'self' 'wasm-unsafe-eval' 'inline-speculation-rules' %s; "
+    "object-src 'self';";
 // For unpacked extensions, we additionally allow the use of localhost files to
 // aid in rapid local development.
 static const char kMinimumUnpackedMV3CSP[] =
     "script-src 'self' 'wasm-unsafe-eval' 'inline-speculation-rules' "
     "http://localhost:* http://127.0.0.1:*; object-src 'self';";
+// The minimum CSP to be used in isolated worlds for unpacked extensions. The
+// placeholder is for the extension's dynamic URL.
+constexpr char kMinimumUnpackedMV3IsolatedWorldCSPTemplate[] =
+    "script-src 'self' 'wasm-unsafe-eval' 'inline-speculation-rules' "
+    "http://localhost:* http://127.0.0.1:* %s; object-src 'self';";
 
 #define PLATFORM_APP_LOCAL_CSP_SOURCES "'self' blob: filesystem: data:"
 
@@ -171,11 +182,23 @@ const std::string* CSPInfo::GetMinimumCSPToAppend(
 }
 
 // static
-const std::string* CSPInfo::GetIsolatedWorldCSP(const Extension& extension) {
+std::optional<std::string> CSPInfo::GetIsolatedWorldCSP(
+    const Extension& extension) {
   if (extension.manifest_version() >= 3) {
-    // The isolated world will use its own CSP which blocks remotely hosted
-    // code.
-    return GetMinimumMV3CSPForExtension(extension);
+    std::string isolated_world_csp;
+    // Note: base::StringPrintf() requires the template string to be constexpr,
+    // so we can't put the template in a temporary const char* with a ternary
+    // to avoid the repeated StringPrintf() calls.
+    if (Manifest::IsUnpackedLocation(extension.location())) {
+      isolated_world_csp =
+          base::StringPrintf(kMinimumUnpackedMV3IsolatedWorldCSPTemplate,
+                             extension.dynamic_url().spec().c_str());
+    } else {
+      isolated_world_csp =
+          base::StringPrintf(kMinimumMV3IsolatedWorldCSPTemplate,
+                             extension.dynamic_url().spec().c_str());
+    }
+    return isolated_world_csp;
   }
 
   Manifest::Type type = extension.GetType();
@@ -184,11 +207,11 @@ const std::string* CSPInfo::GetIsolatedWorldCSP(const Extension& extension) {
                                type == Manifest::TYPE_LEGACY_PACKAGED_APP;
   if (!bypass_main_world_csp) {
     // The isolated world will use the main world CSP.
-    return nullptr;
+    return std::nullopt;
   }
 
   // The isolated world will bypass the main world CSP.
-  return &base::EmptyString();
+  return std::string();
 }
 
 // static

extensions/common/manifest_handlers/csp_info.h

@@ -5,6 +5,7 @@
 #ifndef EXTENSIONS_COMMON_MANIFEST_HANDLERS_CSP_INFO_H_
 #define EXTENSIONS_COMMON_MANIFEST_HANDLERS_CSP_INFO_H_
 
+#include <optional>
 #include <string>
 #include <string_view>
 
@@ -43,8 +44,11 @@ struct CSPInfo : public Extension::ManifestData {
       const std::string& relative_path);
 
   // Returns the Content Security Policy to be used for extension isolated
-  // worlds or null if there is no defined CSP.
-  static const std::string* GetIsolatedWorldCSP(const Extension& extension);
+  // worlds or nullopt if there is no defined CSP.
+  // Note that a non-nullopt, empty string is different from a nullopt result,
+  // since an empty CSP permits everything.
+  static std::optional<std::string> GetIsolatedWorldCSP(
+      const Extension& extension);
 
   // Returns the extension's Content Security Policy for the sandboxed pages.
   static const std::string& GetSandboxContentSecurityPolicy(

extensions/common/manifest_handlers/csp_info_unittest.cc

@@ -117,7 +117,7 @@ TEST_F(CSPInfoUnitTest, CSPStringKey) {
             CSPInfo::GetExtensionPagesCSP(extension.get()));
 
   // Manifest V2 extensions bypass the main world CSP in their isolated worlds.
-  const std::string* isolated_world_csp =
+  std::optional<std::string> isolated_world_csp =
       CSPInfo::GetIsolatedWorldCSP(*extension);
   ASSERT_TRUE(isolated_world_csp);
   EXPECT_TRUE(isolated_world_csp->empty());
@@ -375,10 +375,14 @@ TEST_F(CSPInfoUnitTest, CSPDictionaryMandatoryForV3) {
         LoadAndExpectSuccess(filename, mojom::ManifestLocation::kInternal);
     ASSERT_TRUE(extension);
 
-    const std::string* isolated_world_csp =
+    std::optional<std::string> isolated_world_csp =
         CSPInfo::GetIsolatedWorldCSP(*extension);
     ASSERT_TRUE(isolated_world_csp);
-    EXPECT_EQ(CSPHandler::GetMinimumMV3CSPForTesting(), *isolated_world_csp);
+    std::string expected_csp = base::StringPrintf(
+        "script-src 'self' 'wasm-unsafe-eval' 'inline-speculation-rules' "
+        "%s; object-src 'self';",
+        extension->dynamic_url().spec().c_str());
+    EXPECT_EQ(expected_csp, *isolated_world_csp);
 
     EXPECT_EQ(kDefaultSandboxedPageCSP,
               CSPInfo::GetSandboxContentSecurityPolicy(extension.get()));
@@ -399,11 +403,14 @@ TEST_F(CSPInfoUnitTest, CSPDictionaryMandatoryForV3) {
         LoadAndExpectSuccess(filename, mojom::ManifestLocation::kUnpacked);
     ASSERT_TRUE(extension);
 
-    const std::string* isolated_world_csp =
+    std::optional<std::string> isolated_world_csp =
         CSPInfo::GetIsolatedWorldCSP(*extension);
     ASSERT_TRUE(isolated_world_csp);
-    EXPECT_EQ(CSPHandler::GetMinimumUnpackedMV3CSPForTesting(),
-              *isolated_world_csp);
+    std::string expected_csp = base::StringPrintf(
+        "script-src 'self' 'wasm-unsafe-eval' 'inline-speculation-rules' "
+        "http://localhost:* http://127.0.0.1:* %s; object-src 'self';",
+        extension->dynamic_url().spec().c_str());
+    EXPECT_EQ(expected_csp, *isolated_world_csp);
 
     EXPECT_EQ(kDefaultSandboxedPageCSP,
               CSPInfo::GetSandboxContentSecurityPolicy(extension.get()));

extensions/renderer/extension_injection_host.cc

@@ -40,7 +40,11 @@ std::unique_ptr<const InjectionHost> ExtensionInjectionHost::Create(
 }
 
 const std::string* ExtensionInjectionHost::GetContentSecurityPolicy() const {
-  return CSPInfo::GetIsolatedWorldCSP(*extension_);
+  if (!isolated_world_csp_) {
+    isolated_world_csp_ = CSPInfo::GetIsolatedWorldCSP(*extension_);
+  }
+
+  return &(isolated_world_csp_.value());
 }
 
 const GURL& ExtensionInjectionHost::url() const {

extensions/renderer/extension_injection_host.h

@@ -5,6 +5,9 @@
 #ifndef EXTENSIONS_RENDERER_EXTENSION_INJECTION_HOST_H_
 #define EXTENSIONS_RENDERER_EXTENSION_INJECTION_HOST_H_
 
+#include <optional>
+#include <string>
+
 #include "base/memory/raw_ptr.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_id.h"
@@ -40,6 +43,10 @@ class ExtensionInjectionHost : public InjectionHost {
       bool is_declarative) const override;
 
   raw_ptr<const Extension, DanglingUntriaged> extension_;
+
+  // The isolated world CSP, cached to avoid duplication. Mutable as it is
+  // lazily instantiated.
+  mutable std::optional<std::string> isolated_world_csp_;
 };
 
 }  // namespace extesions