From f2d0be1b9af4e97521a6cc7b818e53e5912731b7 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Sun, 15 Jun 2025 16:54:41 +0200 Subject: [PATCH] script: Fix check for document root when targeting CSP events (#37474) The check was incorrect, where it was never matching and always discarding the element. Instead, we should check the owner document, which is the shadow-including root of the node. Part of #4577 --------- Signed-off-by: Tim van der Lippe --- components/script/dom/globalscope.rs | 19 +++++++++++-------- components/script/dom/htmliframeelement.rs | 2 +- components/script/script_thread.rs | 2 +- components/script/security_manager.rs | 2 +- .../script-sample-no-opt-in.html.ini | 7 ------- .../script-sample.html.ini | 6 +----- .../style-sample-no-opt-in.html.ini | 4 ---- .../style-sample.html.ini | 4 ---- .../targeting.html.ini | 3 --- 9 files changed, 15 insertions(+), 34 deletions(-) delete mode 100644 tests/wpt/meta/content-security-policy/securitypolicyviolation/script-sample-no-opt-in.html.ini delete mode 100644 tests/wpt/meta/content-security-policy/securitypolicyviolation/style-sample-no-opt-in.html.ini delete mode 100644 tests/wpt/meta/content-security-policy/securitypolicyviolation/style-sample.html.ini diff --git a/components/script/dom/globalscope.rs b/components/script/dom/globalscope.rs index 6c3a92136af..20e482c132f 100644 --- a/components/script/dom/globalscope.rs +++ b/components/script/dom/globalscope.rs @@ -121,7 +121,7 @@ use crate::dom::htmlscriptelement::{ScriptId, SourceCode}; use crate::dom::imagebitmap::ImageBitmap; use crate::dom::messageevent::MessageEvent; use crate::dom::messageport::MessagePort; -use crate::dom::node::Node; +use crate::dom::node::{Node, NodeTraits}; use crate::dom::paintworkletglobalscope::PaintWorkletGlobalScope; use crate::dom::performance::Performance; use crate::dom::performanceobserver::VALID_ENTRY_TYPES; @@ -2934,7 +2934,11 @@ impl GlobalScope { is_js_evaluation_allowed == CheckResult::Allowed } - pub(crate) fn should_navigation_request_be_blocked(&self, load_data: &LoadData) -> bool { + pub(crate) fn should_navigation_request_be_blocked( + &self, + load_data: &LoadData, + element: Option<&Element>, + ) -> bool { let Some(csp_list) = self.get_csp_list() else { return false; }; @@ -2956,7 +2960,7 @@ impl GlobalScope { let (result, violations) = csp_list.should_navigation_request_be_blocked(&request, NavigationCheckType::Other); - self.report_csp_violations(violations, None); + self.report_csp_violations(violations, element); result == CheckResult::Blocked } @@ -3627,11 +3631,10 @@ impl GlobalScope { // Step 3.1: If target is not null, and global is a Window, // and target’s shadow-including root is not global’s associated Document, set target to null. if let Some(window) = self.downcast::() { - if !window - .Document() - .upcast::() - .is_shadow_including_inclusive_ancestor_of(event_target.upcast()) - { + // If a node is connected, its owner document is always the shadow-including root. + // If it isn't connected, then it also doesn't have a corresponding document, hence + // it can't be this document. + if event_target.upcast::().owner_document() != window.Document() { return None; } } diff --git a/components/script/dom/htmliframeelement.rs b/components/script/dom/htmliframeelement.rs index fef8c53db0d..a0c837fc020 100644 --- a/components/script/dom/htmliframeelement.rs +++ b/components/script/dom/htmliframeelement.rs @@ -168,7 +168,7 @@ impl HTMLIFrameElement { if let Some(window_proxy) = window_proxy { if document .global() - .should_navigation_request_be_blocked(&load_data) + .should_navigation_request_be_blocked(&load_data, Some(self.upcast())) { return; } diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 2bcb1d23056..0b1f1680a2f 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -627,7 +627,7 @@ impl ScriptThread { if let Some(window) = trusted_global.root().downcast::() { // Step 5: If the result of should navigation request of type be blocked by // Content Security Policy? given request and cspNavigationType is "Blocked", then return. [CSP] - if trusted_global.root().should_navigation_request_be_blocked(&load_data) { + if trusted_global.root().should_navigation_request_be_blocked(&load_data, None) { return; } if ScriptThread::check_load_origin(&load_data.load_origin, &window.get_url().origin()) { diff --git a/components/script/security_manager.rs b/components/script/security_manager.rs index ae6b4543285..fb7e78a8862 100644 --- a/components/script/security_manager.rs +++ b/components/script/security_manager.rs @@ -59,7 +59,7 @@ impl CSPViolationReportTask { &self.global.root(), Atom::from("securitypolicyviolation"), EventBubbles::Bubbles, - EventCancelable::Cancelable, + EventCancelable::NotCancelable, EventComposed::Composed, &self.violation_report.clone().convert(), can_gc, diff --git a/tests/wpt/meta/content-security-policy/securitypolicyviolation/script-sample-no-opt-in.html.ini b/tests/wpt/meta/content-security-policy/securitypolicyviolation/script-sample-no-opt-in.html.ini deleted file mode 100644 index 409022079e0..00000000000 --- a/tests/wpt/meta/content-security-policy/securitypolicyviolation/script-sample-no-opt-in.html.ini +++ /dev/null @@ -1,7 +0,0 @@ -[script-sample-no-opt-in.html] - expected: TIMEOUT - [JavaScript URLs in iframes should not have a sample.] - expected: TIMEOUT - - [Inline event handlers should not have a sample.] - expected: TIMEOUT diff --git a/tests/wpt/meta/content-security-policy/securitypolicyviolation/script-sample.html.ini b/tests/wpt/meta/content-security-policy/securitypolicyviolation/script-sample.html.ini index 8723775f27e..25221214887 100644 --- a/tests/wpt/meta/content-security-policy/securitypolicyviolation/script-sample.html.ini +++ b/tests/wpt/meta/content-security-policy/securitypolicyviolation/script-sample.html.ini @@ -1,7 +1,3 @@ [script-sample.html] - expected: TIMEOUT [JavaScript URLs in iframes should have a sample.] - expected: TIMEOUT - - [Inline event handlers should have a sample.] - expected: TIMEOUT + expected: FAIL diff --git a/tests/wpt/meta/content-security-policy/securitypolicyviolation/style-sample-no-opt-in.html.ini b/tests/wpt/meta/content-security-policy/securitypolicyviolation/style-sample-no-opt-in.html.ini deleted file mode 100644 index ef89e7cb1a9..00000000000 --- a/tests/wpt/meta/content-security-policy/securitypolicyviolation/style-sample-no-opt-in.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[style-sample-no-opt-in.html] - expected: TIMEOUT - [Inline style attributes should not have a sample.] - expected: TIMEOUT diff --git a/tests/wpt/meta/content-security-policy/securitypolicyviolation/style-sample.html.ini b/tests/wpt/meta/content-security-policy/securitypolicyviolation/style-sample.html.ini deleted file mode 100644 index 63e278182d8..00000000000 --- a/tests/wpt/meta/content-security-policy/securitypolicyviolation/style-sample.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[style-sample.html] - expected: TIMEOUT - [Inline style attributes should have a sample.] - expected: TIMEOUT diff --git a/tests/wpt/meta/content-security-policy/securitypolicyviolation/targeting.html.ini b/tests/wpt/meta/content-security-policy/securitypolicyviolation/targeting.html.ini index 952c5185dd8..dd8953a3761 100644 --- a/tests/wpt/meta/content-security-policy/securitypolicyviolation/targeting.html.ini +++ b/tests/wpt/meta/content-security-policy/securitypolicyviolation/targeting.html.ini @@ -8,6 +8,3 @@ [Correct targeting inside shadow tree (inline handler).] expected: TIMEOUT - - [Elements created in this document, but pushed into a same-origin frame trigger on that frame's document, not on this frame's document.] - expected: TIMEOUT