From: uazo Date: Sun, 14 Apr 2024 07:35:57 +0000 Subject: Revert remove AllowScript content settings per secondary url https://chromium-review.googlesource.com/c/chromium/src/+/5307559 --- .../worker_content_settings_client.cc | 35 +++++++++++++++++ .../renderer/worker_content_settings_client.h | 2 + .../renderer/content_settings_agent_impl.cc | 38 ++++++++++++++++++- .../renderer/content_settings_agent_impl.h | 6 +++ .../platform/web_content_settings_client.h | 12 ++++++ .../renderer/core/frame/local_dom_window.cc | 6 ++- .../blink/renderer/core/frame/local_frame.cc | 13 ------- .../blink/renderer/core/frame/local_frame.h | 6 --- .../renderer/core/html/html_meta_element.cc | 4 +- .../core/loader/base_fetch_context.cc | 2 +- .../renderer/core/loader/base_fetch_context.h | 2 +- .../core/loader/frame_fetch_context.cc | 30 ++++++++++----- .../core/loader/frame_fetch_context.h | 13 ++++++- .../core/loader/worker_fetch_context.cc | 12 ++++-- .../core/loader/worker_fetch_context.h | 2 +- 15 files changed, 144 insertions(+), 39 deletions(-) diff --git a/chrome/renderer/worker_content_settings_client.cc b/chrome/renderer/worker_content_settings_client.cc --- a/chrome/renderer/worker_content_settings_client.cc +++ b/chrome/renderer/worker_content_settings_client.cc @@ -117,6 +117,41 @@ bool WorkerContentSettingsClient::AllowRunningInsecureContent( return true; } +bool WorkerContentSettingsClient::AllowScriptFromSource( + bool enabled_per_settings, + const blink::WebURL& script_url) { + bool allow = enabled_per_settings; + if (allow && content_setting_rules_) { + GURL top_frame_origin_url = top_frame_origin_.GetURL(); + // Allow DevTools to run worker scripts. + if (top_frame_origin_url.SchemeIs(content::kChromeDevToolsScheme)) + return true; + for (ContentSettingRuleSource& info : content_setting_rules_->settings_rules) { + if (info.type == (int)ContentSettingsType::JAVASCRIPT) { + for (const auto& rule : info.rules) { + // The primary pattern was already matched in the browser process (see + // PageSpecificContentSettings::ReadyToCommitNavigation), so we only need + // to match the secondary pattern here. + if (rule.secondary_pattern.Matches(script_url)) { + allow = rule.GetContentSetting() != CONTENT_SETTING_BLOCK; + break; + } + } + break; + } + } + } + + if (!allow) { + EnsureContentSettingsManager(); + content_settings_manager_->OnContentBlocked( + frame_token_, ContentSettingsType::JAVASCRIPT); + return false; + } + + return true; +} + bool WorkerContentSettingsClient::ShouldAutoupgradeMixedContent() { if (content_setting_rules_) { if (content_setting_rules_->mixed_content_rules.size() > 0) diff --git a/chrome/renderer/worker_content_settings_client.h b/chrome/renderer/worker_content_settings_client.h --- a/chrome/renderer/worker_content_settings_client.h +++ b/chrome/renderer/worker_content_settings_client.h @@ -37,6 +37,8 @@ class WorkerContentSettingsClient : public blink::WebContentSettingsClient { bool AllowStorageAccessSync(StorageType storage_type) override; bool AllowRunningInsecureContent(bool allowed_per_settings, const blink::WebURL& url) override; + bool AllowScriptFromSource(bool enabled_per_settings, + const blink::WebURL& script_url) override; bool ShouldAutoupgradeMixedContent() override; private: diff --git a/components/content_settings/renderer/content_settings_agent_impl.cc b/components/content_settings/renderer/content_settings_agent_impl.cc --- a/components/content_settings/renderer/content_settings_agent_impl.cc +++ b/components/content_settings/renderer/content_settings_agent_impl.cc @@ -161,7 +161,7 @@ void ContentSettingsAgentImpl::DidCommitProvisionalLoad( // `allowPlugins()` is called for the new page so that these functions can // correctly detect that a piece of content flipped from "not blocked" to // "blocked". - ClearBlockedContentSettings(); + ClearBlockedContentSettings(); // do not remove blink::WebLocalFrame* frame = render_frame()->GetWebFrame(); if (frame->Parent()) @@ -373,6 +373,41 @@ bool ContentSettingsAgentImpl::AllowImage(bool enabled_per_settings, return allow; } +bool ContentSettingsAgentImpl::AllowScript(bool enabled_per_settings) { + if (!enabled_per_settings) + return false; + + blink::WebLocalFrame* frame = render_frame()->GetWebFrame(); + const auto it = cached_script_permissions_.find(frame); + if (it != cached_script_permissions_.end()) + return it->second; + + // Evaluate the content setting rules before + // IsAllowlistedForContentSettings(); if there is only the default rule + // allowing all scripts, it's quicker this way. + bool allow = AllowContentSetting( + ContentSettingsType::JAVASCRIPT, enabled_per_settings); + + allow = allow || IsAllowlistedForContentSettings(); + cached_script_permissions_[frame] = allow; + return allow; +} + +bool ContentSettingsAgentImpl::AllowScriptFromSource( + bool enabled_per_settings, + const blink::WebURL& script_url) { + if (!enabled_per_settings) + return false; + + bool allow = true; + if (content_setting_rules_) { + allow = AllowContentSetting( + ContentSettingsType::JAVASCRIPT, script_url, enabled_per_settings); + } + allow = allow || IsAllowlistedForContentSettings(); + return allow; +} + bool ContentSettingsAgentImpl::AllowReadFromClipboard() { return delegate_->AllowReadFromClipboard(); } @@ -427,6 +462,7 @@ void ContentSettingsAgentImpl::DidNotAllowImage() { void ContentSettingsAgentImpl::ClearBlockedContentSettings() { content_blocked_.clear(); cached_storage_permissions_.clear(); + cached_script_permissions_.clear(); } } // namespace content_settings diff --git a/components/content_settings/renderer/content_settings_agent_impl.h b/components/content_settings/renderer/content_settings_agent_impl.h --- a/components/content_settings/renderer/content_settings_agent_impl.h +++ b/components/content_settings/renderer/content_settings_agent_impl.h @@ -85,6 +85,9 @@ class ContentSettingsAgentImpl bool AllowStorageAccessSync(StorageType type) override; bool AllowImage(bool enabled_per_settings, const blink::WebURL& image_url) override; + bool AllowScript(bool enabled_per_settings) override; + bool AllowScriptFromSource(bool enabled_per_settings, + const blink::WebURL& script_url) override; bool AllowReadFromClipboard() override; bool AllowWriteToClipboard() override; void DidNotAllowImage() override; @@ -151,6 +154,9 @@ class ContentSettingsAgentImpl using StoragePermissionsKey = std::pair; base::flat_map cached_storage_permissions_; + // Caches the result of AllowScript. + base::flat_map cached_script_permissions_; + std::unique_ptr delegate_; mojo::AssociatedReceiverSet receivers_; diff --git a/third_party/blink/public/platform/web_content_settings_client.h b/third_party/blink/public/platform/web_content_settings_client.h --- a/third_party/blink/public/platform/web_content_settings_client.h +++ b/third_party/blink/public/platform/web_content_settings_client.h @@ -60,6 +60,18 @@ class WebContentSettingsClient { return enabled_per_settings; } + // Controls whether scripts are allowed to execute for this frame. + virtual bool AllowScript(bool enabled_per_settings) { + return enabled_per_settings; + } + + // Controls whether scripts loaded from the given URL are allowed to execute + // for this frame. + virtual bool AllowScriptFromSource(bool enabled_per_settings, + const WebURL& script_url) { + return enabled_per_settings; + } + // Controls whether insecure scripts are allowed to execute for this frame. virtual bool AllowRunningInsecureContent(bool enabled_per_settings, const WebURL&) { diff --git a/third_party/blink/renderer/core/frame/local_dom_window.cc b/third_party/blink/renderer/core/frame/local_dom_window.cc --- a/third_party/blink/renderer/core/frame/local_dom_window.cc +++ b/third_party/blink/renderer/core/frame/local_dom_window.cc @@ -51,6 +51,7 @@ #include "third_party/blink/public/platform/task_type.h" #include "third_party/blink/public/platform/web_string.h" #include "third_party/blink/public/web/web_picture_in_picture_window_options.h" +#include "third_party/blink/renderer/core/loader/frame_fetch_context.h" #include "third_party/blink/renderer/bindings/core/v8/binding_security.h" #include "third_party/blink/renderer/bindings/core/v8/capture_source_location.h" #include "third_party/blink/renderer/bindings/core/v8/isolated_world_csp.h" @@ -487,7 +488,10 @@ bool LocalDOMWindow::CanExecuteScripts( } return false; } - bool script_enabled = GetFrame()->ScriptEnabled(); + bool script_enabled = + FrameFetchContext::AllowScriptFromSourceWithoutNotifying( + Url(), GetFrame()->GetContentSettingsClient(), + GetFrame()->GetSettings()); if (!script_enabled && reason == kAboutToExecuteScript) { WebContentSettingsClient* settings_client = GetFrame()->GetContentSettingsClient(); diff --git a/third_party/blink/renderer/core/frame/local_frame.cc b/third_party/blink/renderer/core/frame/local_frame.cc --- a/third_party/blink/renderer/core/frame/local_frame.cc +++ b/third_party/blink/renderer/core/frame/local_frame.cc @@ -4161,19 +4161,6 @@ bool LocalFrame::IsSameOrigin() { return security_origin->IsSameOriginWith(top_security_origin); } -bool LocalFrame::ScriptEnabled() { - DCHECK(!IsDetached()); - // If this is called in the middle of detach, GetDocumentLoader() might - // already be nullptr. - if (!loader_.GetDocumentLoader()) { - return false; - } - bool allow_script_renderer = GetSettings()->GetScriptEnabled(); - bool allow_script_content_setting = - loader_.GetDocumentLoader()->GetContentSettings()->allow_script; - return allow_script_renderer && allow_script_content_setting; -} - const WebPrintParams& LocalFrame::GetPrintParams() const { // If this fails, it's probably because nobody called StartPrinting(). CHECK(GetDocument()->Printing()); diff --git a/third_party/blink/renderer/core/frame/local_frame.h b/third_party/blink/renderer/core/frame/local_frame.h --- a/third_party/blink/renderer/core/frame/local_frame.h +++ b/third_party/blink/renderer/core/frame/local_frame.h @@ -936,12 +936,6 @@ class CORE_EXPORT LocalFrame final return *v8_local_compile_hints_producer_; } - // Returns whether script is allowed to run for the current frame. This is a - // convenience method that checks both renderer content settings and frame - // settings. - // Can only be called while the frame is not detached. - bool ScriptEnabled(); - const WebPrintParams& GetPrintParams() const; // Returns the `Frame` for which `provisional_frame_ == this`. May only be diff --git a/third_party/blink/renderer/core/html/html_meta_element.cc b/third_party/blink/renderer/core/html/html_meta_element.cc --- a/third_party/blink/renderer/core/html/html_meta_element.cc +++ b/third_party/blink/renderer/core/html/html_meta_element.cc @@ -811,7 +811,9 @@ void HTMLMetaElement::ProcessMetaCH(Document& document, return; } - if (!frame->ScriptEnabled()) { + if (!FrameFetchContext::AllowScriptFromSourceWithoutNotifying( + document.Url(), frame->GetContentSettingsClient(), + frame->GetSettings())) { // Do not allow configuring client hints if JavaScript is disabled. return; } diff --git a/third_party/blink/renderer/core/loader/base_fetch_context.cc b/third_party/blink/renderer/core/loader/base_fetch_context.cc --- a/third_party/blink/renderer/core/loader/base_fetch_context.cc +++ b/third_party/blink/renderer/core/loader/base_fetch_context.cc @@ -290,7 +290,7 @@ BaseFetchContext::CanRequestInternal( } if (type == ResourceType::kScript) { - if (!AllowScript()) { + if (!AllowScriptFromSource(url)) { // TODO(estark): Use a different ResourceRequestBlockedReason here, since // this check has nothing to do with CSP. https://crbug.com/600795 return ResourceRequestBlockedReason::kCSP; diff --git a/third_party/blink/renderer/core/loader/base_fetch_context.h b/third_party/blink/renderer/core/loader/base_fetch_context.h --- a/third_party/blink/renderer/core/loader/base_fetch_context.h +++ b/third_party/blink/renderer/core/loader/base_fetch_context.h @@ -108,7 +108,7 @@ class CORE_EXPORT BaseFetchContext : public FetchContext { : fetcher_properties_(properties), console_logger_(logger) {} // Used for security checks. - virtual bool AllowScript() const = 0; + virtual bool AllowScriptFromSource(const KURL&) const = 0; // Note: subclasses are expected to override following methods. // Used in the default implementation for CanRequest, CanFollowRedirect diff --git a/third_party/blink/renderer/core/loader/frame_fetch_context.cc b/third_party/blink/renderer/core/loader/frame_fetch_context.cc --- a/third_party/blink/renderer/core/loader/frame_fetch_context.cc +++ b/third_party/blink/renderer/core/loader/frame_fetch_context.cc @@ -635,7 +635,8 @@ void FrameFetchContext::AddClientHintsIfNecessary( // Check if |url| is allowed to run JavaScript. If not, client hints are not // attached to the requests that initiate on the render side. - if (!GetFrame()->ScriptEnabled()) { + if (!AllowScriptFromSourceWithoutNotifying( + request.Url(), GetContentSettingsClient(), GetSettings())) { return; } @@ -1100,15 +1101,26 @@ void FrameFetchContext::SetFirstPartyCookie(ResourceRequest& request) { } } -bool FrameFetchContext::AllowScript() const { - bool script_enabled = GetFrame()->ScriptEnabled(); - if (!script_enabled) { - WebContentSettingsClient* settings_client = GetContentSettingsClient(); - if (settings_client) { - settings_client->DidNotAllowScript(); - } +bool FrameFetchContext::AllowScriptFromSource(const KURL& url) const { + if (AllowScriptFromSourceWithoutNotifying(url, GetContentSettingsClient(), + GetSettings())) { + return true; } - return script_enabled; + WebContentSettingsClient* settings_client = GetContentSettingsClient(); + if (settings_client) + settings_client->DidNotAllowScript(); + return false; +} + +// static +bool FrameFetchContext::AllowScriptFromSourceWithoutNotifying( + const KURL& url, + WebContentSettingsClient* settings_client, + Settings* settings) { + bool allow_script = !settings || settings->GetScriptEnabled(); + if (settings_client) + allow_script = settings_client->AllowScriptFromSource(allow_script, url); + return allow_script; } bool FrameFetchContext::IsFirstPartyOrigin( diff --git a/third_party/blink/renderer/core/loader/frame_fetch_context.h b/third_party/blink/renderer/core/loader/frame_fetch_context.h --- a/third_party/blink/renderer/core/loader/frame_fetch_context.h +++ b/third_party/blink/renderer/core/loader/frame_fetch_context.h @@ -68,6 +68,17 @@ class WebContentSettingsClient; class CORE_EXPORT FrameFetchContext final : public BaseFetchContext, public LoadingBehaviorObserver { public: + // Returns true if execution of scripts from the url are allowed. Compared to + // AllowScriptFromSource(), this method does not generate any + // notification to the `WebContentSettingsClient` that the execution of the + // script was blocked. This method should be called only when there is a need + // to check the settings, and where blocked setting doesn't really imply that + // JavaScript was blocked from being executed. + static bool AllowScriptFromSourceWithoutNotifying( + const KURL& url, + WebContentSettingsClient* settings_client, + Settings* settings); + static ResourceFetcher* CreateFetcherForCommittedDocument(DocumentLoader&, Document&); FrameFetchContext(DocumentLoader& document_loader, @@ -183,7 +194,7 @@ class CORE_EXPORT FrameFetchContext final : public BaseFetchContext, // BaseFetchContext overrides: net::SiteForCookies GetSiteForCookies() const override; SubresourceFilter* GetSubresourceFilter() const override; - bool AllowScript() const override; + bool AllowScriptFromSource(const KURL&) const override; bool ShouldBlockRequestByInspector(const KURL&) const override; void DispatchDidBlockRequest(const ResourceRequest&, const ResourceLoaderOptions&, diff --git a/third_party/blink/renderer/core/loader/worker_fetch_context.cc b/third_party/blink/renderer/core/loader/worker_fetch_context.cc --- a/third_party/blink/renderer/core/loader/worker_fetch_context.cc +++ b/third_party/blink/renderer/core/loader/worker_fetch_context.cc @@ -79,10 +79,14 @@ SubresourceFilter* WorkerFetchContext::GetSubresourceFilter() const { return subresource_filter_.Get(); } -bool WorkerFetchContext::AllowScript() const { - // Script is always allowed in worker fetch contexts, since the fact that - // they're running is already evidence that script is allowed. - return true; +bool WorkerFetchContext::AllowScriptFromSource(const KURL& url) const { + if (!global_scope_->ContentSettingsClient()) { + return true; + } + // If we're on a worker, script should be enabled, so no need to plumb + // Settings::GetScriptEnabled() here. + return global_scope_->ContentSettingsClient()->AllowScriptFromSource(true, + url); } bool WorkerFetchContext::ShouldBlockRequestByInspector(const KURL& url) const { diff --git a/third_party/blink/renderer/core/loader/worker_fetch_context.h b/third_party/blink/renderer/core/loader/worker_fetch_context.h --- a/third_party/blink/renderer/core/loader/worker_fetch_context.h +++ b/third_party/blink/renderer/core/loader/worker_fetch_context.h @@ -48,7 +48,7 @@ class WorkerFetchContext final : public BaseFetchContext { // BaseFetchContext implementation: net::SiteForCookies GetSiteForCookies() const override; SubresourceFilter* GetSubresourceFilter() const override; - bool AllowScript() const override; + bool AllowScriptFromSource(const KURL&) const override; bool ShouldBlockRequestByInspector(const KURL&) const override; void DispatchDidBlockRequest(const ResourceRequest&, const ResourceLoaderOptions&, --