diff --git a/extensions/browser/api/user_scripts/user_scripts_api.cc b/extensions/browser/api/user_scripts/user_scripts_api.cc index c5f00b8940e915..46ef2c00622177 100644 --- a/extensions/browser/api/user_scripts/user_scripts_api.cc +++ b/extensions/browser/api/user_scripts/user_scripts_api.cc @@ -695,18 +695,8 @@ ExtensionFunction::ResponseAction UserScriptsExecuteFunction::Run() { } // Validate injection target. - scripting::InjectionTarget internal_injection_target = + injection_target_ = ConvertToInternalInjectionTarget(std::move(injection_.target)); - ScriptExecutor* script_executor = nullptr; - ScriptExecutor::FrameScope frame_scope = ScriptExecutor::SPECIFIED_FRAMES; - std::set frame_ids; - if (!scripting::CanAccessTarget( - *extension()->permissions_data(), internal_injection_target, - browser_context(), include_incognito_information(), &script_executor, - &frame_scope, &frame_ids, &error)) { - return RespondNow(Error(std::move(error))); - } - // Retrieve injection sources in order. Since file sources need to be loaded, // we use a nullopt for placeholder in the source list. After files are // successfully loaded, they should be converted to js source too. @@ -729,21 +719,18 @@ ExtensionFunction::ResponseAction UserScriptsExecuteFunction::Run() { std::move(file_sources), script_parsing::ContentScriptType::kJs, *extension(), kRequiresLocalization, base::BindOnce(&UserScriptsExecuteFunction::DidLoadResources, this, - script_executor, frame_scope, std::move(frame_ids), std::move(sources)), &error); - } else { - Execute(std::move(sources), script_executor, frame_scope, frame_ids, - &error); + return RespondLater(); } + if (!Execute(std::move(sources), &error)) { + return RespondNow(Error(std::move(error))); + } return RespondLater(); } void UserScriptsExecuteFunction::DidLoadResources( - ScriptExecutor* script_executor, - ScriptExecutor::FrameScope frame_scope, - std::set frame_ids, std::vector> sources, std::vector file_sources, std::optional load_error) { @@ -773,15 +760,30 @@ void UserScriptsExecuteFunction::DidLoadResources( CHECK_EQ(file_index, static_cast(file_sources.size())); std::string error; - Execute(std::move(sources), script_executor, frame_scope, frame_ids, &error); + if (!Execute(std::move(sources), &error)) { + Respond(Error(std::move(error))); + } } -void UserScriptsExecuteFunction::Execute( +bool UserScriptsExecuteFunction::Execute( std::vector> sources, - ScriptExecutor* script_executor, - ScriptExecutor::FrameScope frame_scope, - std::set frame_ids, std::string* error) { + // Resolve and validate the injection target now (mirroring + // ScriptingExecuteScriptFunction::Execute()), so that the per-frame + // permission check observes the same RenderFrameHosts that + // ScriptExecutor::Handler will resolve and pass to + // ScriptInjectionTracker::WillExecuteCode() — not stale ones captured + // before the asynchronous file load in Run(). + ScriptExecutor* script_executor = nullptr; + ScriptExecutor::FrameScope frame_scope = ScriptExecutor::SPECIFIED_FRAMES; + std::set frame_ids; + if (!scripting::CanAccessTarget( + *extension()->permissions_data(), injection_target_, + browser_context(), include_incognito_information(), &script_executor, + &frame_scope, &frame_ids, error)) { + return false; + } + mojom::ExecutionWorld execution_world = ConvertExecutionWorld(injection_.world); std::optional execution_world_id = injection_.world_id; @@ -799,6 +801,7 @@ void UserScriptsExecuteFunction::Execute( execution_world_id, script_executor, frame_scope, frame_ids, inject_immediately, user_gesture(), base::BindOnce(&UserScriptsExecuteFunction::OnScriptExecuted, this)); + return true; } void UserScriptsExecuteFunction::OnScriptExecuted( diff --git a/extensions/browser/api/user_scripts/user_scripts_api.h b/extensions/browser/api/user_scripts/user_scripts_api.h index 0d77d7ec416e9d..63dfa0defa626c 100644 --- a/extensions/browser/api/user_scripts/user_scripts_api.h +++ b/extensions/browser/api/user_scripts/user_scripts_api.h @@ -115,24 +115,21 @@ class UserScriptsExecuteFunction : public ExtensionFunction { ~UserScriptsExecuteFunction() override = default; // Called when the resource files to be injected has been loaded. - void DidLoadResources(ScriptExecutor* script_executor, - ScriptExecutor::FrameScope frame_scope, - std::set frame_ids, - std::vector> sources, + void DidLoadResources(std::vector> sources, std::vector file_sources, std::optional load_error); - // Triggers the execution of `sources` in the appropriate context. - void Execute(std::vector> sources, - ScriptExecutor* script_executor, - ScriptExecutor::FrameScope frame_scope, - std::set frame_ids, + // Triggers the execution of `sources` in the appropriate context. Re-resolves + // and re-validates the injection target so that frame_ids captured before the + // async file load are not used after a cross-origin navigation. + bool Execute(std::vector> sources, std::string* error); // Invoked when script execution is complete. void OnScriptExecuted(std::vector frame_results); api::user_scripts::UserScriptInjection injection_; + scripting::InjectionTarget injection_target_; }; class UserScriptsConfigureWorldFunction : public ExtensionFunction { diff --git a/extensions/browser/scripting_utils.cc b/extensions/browser/scripting_utils.cc index cb04eba0c4553b..68ae3369792edc 100644 --- a/extensions/browser/scripting_utils.cc +++ b/extensions/browser/scripting_utils.cc @@ -182,6 +182,8 @@ InjectionTarget::InjectionTarget() : tab_id(-1) {} InjectionTarget::InjectionTarget(InjectionTarget&& other) = default; +InjectionTarget& InjectionTarget::operator=(InjectionTarget&& other) = default; + InjectionTarget::~InjectionTarget() = default; InjectedFileSource::InjectedFileSource(std::string file_name, std::string data) diff --git a/extensions/browser/scripting_utils.h b/extensions/browser/scripting_utils.h index 837c0c7b88ed85..52f02ccf1daf41 100644 --- a/extensions/browser/scripting_utils.h +++ b/extensions/browser/scripting_utils.h @@ -29,6 +29,7 @@ namespace extensions::scripting { struct InjectionTarget { InjectionTarget(); InjectionTarget(InjectionTarget&& other); + InjectionTarget& operator=(InjectionTarget&& other); ~InjectionTarget(); // Whether the script should inject into all frames within the tab.