From 84f478a47a7199a62850f09e7f3e7c8358ad9f4a Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Wed, 27 Aug 2025 23:53:18 +0200 Subject: [PATCH] Implement trusted types processing for JavaScript URL (#38623) We pass in the new trait implementation to process the value, which the CSP crate calls in its implementation. Additionally, since the request url can change, we need to propagate that to load_data as well. This also avoids a crash when a discarded browsing context is accessed while navigating the iframes in the WPT tests. This is a known issue, but hampers investigation into actual Trusted Types support. All tests using iframes don't work, as they don't have the correct browsing context. The other tests do work, but some fail on header ascii parsing (#36801) or error while handling errors. That last one I don't understand based on the current code and I would need to do a deep-dive in the existing code to understand better what's going on. Part of #36258 Part of #37920 --------- Signed-off-by: Tim van der Lippe --- Cargo.lock | 2 +- components/script/dom/csp.rs | 32 ++++++++++++--- components/script/dom/htmliframeelement.rs | 15 +++---- components/script/script_thread.rs | 40 +++++++++++++++---- .../navigate-to-javascript-url-002.html.ini | 3 -- .../navigate-to-javascript-url-003.html.ini | 1 + .../navigate-to-javascript-url-005.html.ini | 3 -- ...avigate-to-javascript-url-006.sub.html.ini | 3 -- ...ate-to-javascript-url-csp-headers.html.ini | 6 --- .../trusted-types-navigation.html.ini | 6 --- 10 files changed, 66 insertions(+), 45 deletions(-) delete mode 100644 tests/wpt/meta/trusted-types/navigate-to-javascript-url-002.html.ini delete mode 100644 tests/wpt/meta/trusted-types/navigate-to-javascript-url-005.html.ini delete mode 100644 tests/wpt/meta/trusted-types/navigate-to-javascript-url-006.sub.html.ini diff --git a/Cargo.lock b/Cargo.lock index e40f923304d..e1fed0c9af1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1629,7 +1629,7 @@ dependencies = [ [[package]] name = "content-security-policy" version = "0.5.4" -source = "git+https://github.com/servo/rust-content-security-policy?branch=servo-csp#e8d4883f9a9349e602465f31a780bc6d70b98528" +source = "git+https://github.com/servo/rust-content-security-policy?branch=servo-csp#cf67beb96db9244ab6956a4da61dbe83384d5cd7" dependencies = [ "base64 0.22.1", "bitflags 2.9.3", diff --git a/components/script/dom/csp.rs b/components/script/dom/csp.rs index c113947d621..1fe2f83680c 100644 --- a/components/script/dom/csp.rs +++ b/components/script/dom/csp.rs @@ -19,13 +19,16 @@ use js::rust::describe_scripted_caller; use log::warn; use crate::dom::bindings::codegen::Bindings::WindowBinding::WindowMethods; +use crate::dom::bindings::codegen::UnionTypes::TrustedScriptOrString; use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::refcounted::Trusted; use crate::dom::csppolicyviolationreport::CSPViolationReportBuilder; use crate::dom::element::Element; use crate::dom::globalscope::GlobalScope; use crate::dom::node::{Node, NodeTraits}; +use crate::dom::trustedscript::TrustedScript; use crate::dom::window::Window; +use crate::script_runtime::CanGc; use crate::security_manager::CSPViolationReportTask; pub(crate) trait CspReporting { @@ -34,8 +37,9 @@ pub(crate) trait CspReporting { fn should_navigation_request_be_blocked( &self, global: &GlobalScope, - load_data: &LoadData, + load_data: &mut LoadData, element: Option<&Element>, + can_gc: CanGc, ) -> bool; fn should_elements_inline_type_behavior_be_blocked( &self, @@ -96,13 +100,14 @@ impl CspReporting for Option { fn should_navigation_request_be_blocked( &self, global: &GlobalScope, - load_data: &LoadData, + load_data: &mut LoadData, element: Option<&Element>, + can_gc: CanGc, ) -> bool { let Some(csp_list) = self else { return false; }; - let request = Request { + let mut request = Request { url: load_data.url.clone().into_url(), origin: match &load_data.load_origin { LoadOrigin::Script(immutable_origin) => immutable_origin.clone().into_url_origin(), @@ -117,8 +122,25 @@ impl CspReporting for Option { parser_metadata: ParserMetadata::None, }; // TODO: set correct navigation check type for form submission if applicable - let (result, violations) = - csp_list.should_navigation_request_be_blocked(&request, NavigationCheckType::Other); + let (result, violations) = csp_list.should_navigation_request_be_blocked( + &mut request, + NavigationCheckType::Other, + |script_source| { + // Step 4. Let convertedScriptSource be the result of executing + // Process value with a default policy algorithm, with the following arguments: + TrustedScript::get_trusted_script_compliant_string( + global, + TrustedScriptOrString::String(script_source.into()), + "Location href", + can_gc, + ) + .ok() + .map(|s| s.into()) + }, + ); + + // In case trusted types processing has changed the Javascript contents + load_data.url = request.url.into(); global.report_csp_violations(violations, element, None); diff --git a/components/script/dom/htmliframeelement.rs b/components/script/dom/htmliframeelement.rs index 444082d6da3..e675668d655 100644 --- a/components/script/dom/htmliframeelement.rs +++ b/components/script/dom/htmliframeelement.rs @@ -34,7 +34,6 @@ use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::reflector::DomGlobal; use crate::dom::bindings::root::{DomRoot, LayoutDom, MutNullableDom}; use crate::dom::bindings::str::{DOMString, USVString}; -use crate::dom::csp::CspReporting; use crate::dom::document::{Document, determine_policy_for_token}; use crate::dom::domtokenlist::DOMTokenList; use crate::dom::element::{ @@ -167,19 +166,15 @@ impl HTMLIFrameElement { if load_data.url.scheme() == "javascript" { let window_proxy = self.GetContentWindow(); if let Some(window_proxy) = window_proxy { - let global = &document.global(); - if global.get_csp_list().should_navigation_request_be_blocked( - global, - &load_data, + if !ScriptThread::navigate_to_javascript_url( + &document.global(), + &window_proxy.global(), + &mut load_data, Some(self.upcast()), + can_gc, ) { return; } - // Important re security. See https://github.com/servo/servo/issues/23373 - if ScriptThread::check_load_origin(&load_data.load_origin, &document.url().origin()) - { - ScriptThread::eval_js_url(&window_proxy.global(), &mut load_data, can_gc); - } } } diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 7d80f8d4df4..119c0d77dd3 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -630,15 +630,9 @@ impl ScriptThread { .clone(); let task = task!(navigate_javascript: move || { // Important re security. See https://github.com/servo/servo/issues/23373 - if let Some(window) = trusted_global.root().downcast::() { + if trusted_global.root().is::() { let global = &trusted_global.root(); - // 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 global.get_csp_list().should_navigation_request_be_blocked(global, &load_data, None) { - return; - } - if ScriptThread::check_load_origin(&load_data.load_origin, &window.get_url().origin()) { - ScriptThread::eval_js_url(&trusted_global.root(), &mut load_data, CanGc::note()); + if Self::navigate_to_javascript_url(global, global, &mut load_data, None, CanGc::note()) { sender .send((pipeline_id, ScriptToConstellationMessage::LoadUrl(load_data, history_handling))) .unwrap(); @@ -663,6 +657,36 @@ impl ScriptThread { }); } + /// + pub(crate) fn navigate_to_javascript_url( + global: &GlobalScope, + containing_global: &GlobalScope, + load_data: &mut LoadData, + container: Option<&Element>, + can_gc: CanGc, + ) -> bool { + // Step 3. If initiatorOrigin is not same origin-domain with targetNavigable's active document's origin, then return. + // + // Important re security. See https://github.com/servo/servo/issues/23373 + if !Self::check_load_origin(&load_data.load_origin, &global.get_url().origin()) { + return false; + } + + // 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 global + .get_csp_list() + .should_navigation_request_be_blocked(global, load_data, container, can_gc) + { + return false; + } + + // Step 6. Let newDocument be the result of evaluating a javascript: URL given targetNavigable, + // url, initiatorOrigin, and userInvolvement. + Self::eval_js_url(containing_global, load_data, can_gc); + true + } + pub(crate) fn process_attach_layout(new_layout_info: NewLayoutInfo, origin: MutableOrigin) { with_script_thread(|script_thread| { let pipeline_id = Some(new_layout_info.new_pipeline_id); diff --git a/tests/wpt/meta/trusted-types/navigate-to-javascript-url-002.html.ini b/tests/wpt/meta/trusted-types/navigate-to-javascript-url-002.html.ini deleted file mode 100644 index 9dc0261d301..00000000000 --- a/tests/wpt/meta/trusted-types/navigate-to-javascript-url-002.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[navigate-to-javascript-url-002.html] - [Setting window.location to a javascript: URL with a default policy should execute the JavaScript code modified by that policy.] - expected: FAIL diff --git a/tests/wpt/meta/trusted-types/navigate-to-javascript-url-003.html.ini b/tests/wpt/meta/trusted-types/navigate-to-javascript-url-003.html.ini index 882171fe8a4..9a46b8d8825 100644 --- a/tests/wpt/meta/trusted-types/navigate-to-javascript-url-003.html.ini +++ b/tests/wpt/meta/trusted-types/navigate-to-javascript-url-003.html.ini @@ -1,3 +1,4 @@ [navigate-to-javascript-url-003.html] + expected: ERROR [Setting window.location to a javascript: URL with a default policy that throws should report a CSP violation without rethrowing the exception.] expected: FAIL diff --git a/tests/wpt/meta/trusted-types/navigate-to-javascript-url-005.html.ini b/tests/wpt/meta/trusted-types/navigate-to-javascript-url-005.html.ini deleted file mode 100644 index 5ef82ad1bf1..00000000000 --- a/tests/wpt/meta/trusted-types/navigate-to-javascript-url-005.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[navigate-to-javascript-url-005.html] - [A subframe navigating to a javascript: URL should use the CSP policy associated to its document for pre-navigation check and report a violation when it does not defined a default policy.] - expected: FAIL diff --git a/tests/wpt/meta/trusted-types/navigate-to-javascript-url-006.sub.html.ini b/tests/wpt/meta/trusted-types/navigate-to-javascript-url-006.sub.html.ini deleted file mode 100644 index 835d3599078..00000000000 --- a/tests/wpt/meta/trusted-types/navigate-to-javascript-url-006.sub.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[navigate-to-javascript-url-006.sub.html] - [A cross-origin subframe navigating to a javascript: URL should use the CSP policy associated to its document for pre-navigation check and execute the JavaScript code modified by its default policy.] - expected: FAIL diff --git a/tests/wpt/meta/trusted-types/navigate-to-javascript-url-csp-headers.html.ini b/tests/wpt/meta/trusted-types/navigate-to-javascript-url-csp-headers.html.ini index c4a50eb9ef8..0d1f584a542 100644 --- a/tests/wpt/meta/trusted-types/navigate-to-javascript-url-csp-headers.html.ini +++ b/tests/wpt/meta/trusted-types/navigate-to-javascript-url-csp-headers.html.ini @@ -1,11 +1,5 @@ [navigate-to-javascript-url-csp-headers.html] expected: TIMEOUT - [One enforce require-trusted-types-for 'script' directive: navigation is blocked, violation is reported.] - expected: FAIL - - [One report-only require-trusted-types-for 'script' directive: navigation continues, violation is reported.] - expected: FAIL - [Multiple enforce require-trusted-types-for directives: one violation reported for each require-trusted-types-for 'script', invalid sink groups ignored.] expected: FAIL diff --git a/tests/wpt/meta/trusted-types/trusted-types-navigation.html.ini b/tests/wpt/meta/trusted-types/trusted-types-navigation.html.ini index 1c5656a2cd1..532f97cdaaa 100644 --- a/tests/wpt/meta/trusted-types/trusted-types-navigation.html.ini +++ b/tests/wpt/meta/trusted-types/trusted-types-navigation.html.ini @@ -164,14 +164,8 @@ [Navigate a window via anchor with javascript:-urls in enforcing mode.] expected: FAIL - [Navigate a window via anchor with javascript:-urls w/ default policy in enforcing mode.] - expected: FAIL - [Navigate a window via anchor with javascript:-urls in report-only mode.] expected: FAIL - [Navigate a window via anchor with javascript:-urls w/ default policy in report-only mode.] - expected: FAIL - [Navigate a frame via anchor with javascript:-urls in enforcing mode.] expected: FAIL