diff --git a/moli-renderer-v8/src/context_bootstrap.rs b/moli-renderer-v8/src/context_bootstrap.rs index 1d33aa05fe..1bd804c00b 100644 --- a/moli-renderer-v8/src/context_bootstrap.rs +++ b/moli-renderer-v8/src/context_bootstrap.rs @@ -460,7 +460,6 @@ pub(crate) use self::trusted_types::{ trusted_html_string_or_throw, trusted_html_value_string, trusted_script_string_for_script_element_execution, trusted_script_string_or_type_error, trusted_script_url_string_or_throw, trusted_types_code_generation_check, - trusted_types_code_generation_check_callback, }; pub(crate) use self::url_search_params_runtime::url_search_params_request_body; pub(crate) use self::web_storage::install_storage_aliases_for_window; diff --git a/moli-renderer-v8/src/context_bootstrap/trusted_types.rs b/moli-renderer-v8/src/context_bootstrap/trusted_types.rs index 615a588253..e0dc78b4c1 100644 --- a/moli-renderer-v8/src/context_bootstrap/trusted_types.rs +++ b/moli-renderer-v8/src/context_bootstrap/trusted_types.rs @@ -318,33 +318,6 @@ pub(crate) enum TrustedTypesCodeGenerationCheck { Block, } -pub(crate) fn trusted_types_code_generation_check_callback<'s>( - scope: &mut v8::PinScope<'s, '_>, - source: v8::Local<'s, v8::Value>, - is_code_like: bool, -) -> v8::ModifyCodeGenerationFromStringsResult<'s> { - if trusted_types_eval_is_allowed(scope) && source.is_string() { - return v8::ModifyCodeGenerationFromStringsResult { - codegen_allowed: true, - modified_source: None, - }; - } - let requirements = trusted_types_for_script_requirements(scope); - let (codegen_allowed, modified_source) = - match trusted_types_code_generation_check(scope, source, is_code_like, requirements) { - TrustedTypesCodeGenerationCheck::AllowOriginal => (true, None), - TrustedTypesCodeGenerationCheck::AllowModified(source) => { - let source = v8_string(scope, &source); - (source.is_some(), source) - } - TrustedTypesCodeGenerationCheck::Block => (false, None), - }; - v8::ModifyCodeGenerationFromStringsResult { - codegen_allowed, - modified_source, - } -} - pub(crate) fn trusted_types_code_generation_check<'s>( scope: &mut v8::PinScope<'s, '_>, source: v8::Local<'s, v8::Value>, @@ -1047,18 +1020,6 @@ fn trusted_script_string_for_function_constructor<'s>( None } -fn trusted_types_for_script_requirements( - scope: &mut v8::PinScope<'_, '_>, -) -> TrustedTypesForScriptRequirements { - if let Some(required) = crate::worker::worker_requires_trusted_types_for_script(scope) { - return TrustedTypesForScriptRequirements::enforced_only(required); - } - let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) else { - return TrustedTypesForScriptRequirements::default(); - }; - unsafe { &*host_ptr }.trusted_types_for_script_requirements(scope) -} - fn trusted_types_eval_is_allowed(scope: &mut v8::PinScope<'_, '_>) -> bool { if let Some(allowed) = crate::worker::worker_allows_trusted_types_eval(scope) { return allowed; diff --git a/moli-renderer-v8/src/script_vm.rs b/moli-renderer-v8/src/script_vm.rs index 1b6d08834a..bb4b2a42af 100644 --- a/moli-renderer-v8/src/script_vm.rs +++ b/moli-renderer-v8/src/script_vm.rs @@ -803,6 +803,7 @@ pub(crate) use runtime_script_continuation::RuntimeScriptContinuationBodyEffect; #[cfg(test)] pub(crate) use runtime_script_continuation::RuntimeScriptOwnerAdvance; mod security_policy; +pub(crate) use security_policy::string_code_generation_check_callback; mod service_worker_client_message_body; #[cfg(test)] mod service_worker_client_message_test_support; diff --git a/moli-renderer-v8/src/script_vm/security_policy.rs b/moli-renderer-v8/src/script_vm/security_policy.rs index b47f8ebd51..1319472907 100644 --- a/moli-renderer-v8/src/script_vm/security_policy.rs +++ b/moli-renderer-v8/src/script_vm/security_policy.rs @@ -141,21 +141,35 @@ pub(super) unsafe extern "C" fn wasm_code_generation_check_callback( host.allows_wasm_code_generation_by_csp(scope) } -pub(super) fn string_code_generation_check_callback<'s>( +pub(crate) fn string_code_generation_check_callback<'s>( scope: &mut v8::PinScope<'s, '_>, source: v8::Local<'s, v8::Value>, is_code_like: bool, ) -> v8::ModifyCodeGenerationFromStringsResult<'s> { - let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) else { - return code_generation_result(true, None); - }; - if crate::context_bootstrap::consume_internal_javascript_url_eval(scope) { - return code_generation_result(true, None); - } - let trusted_types_requirements = - unsafe { &*host_ptr }.trusted_types_for_script_requirements(scope); + let worker_trusted_types_requirements = + crate::worker::worker_trusted_types_for_script_requirements(scope); + let (trusted_types_requirements, allow_trusted_types_eval, host_ptr) = + if let Some(requirements) = worker_trusted_types_requirements { + ( + requirements, + crate::worker::worker_allows_trusted_types_eval(scope).unwrap_or(false), + None, + ) + } else { + let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) else { + return code_generation_result(true, None); + }; + if crate::context_bootstrap::consume_internal_javascript_url_eval(scope) { + return code_generation_result(true, None); + } + ( + unsafe { &*host_ptr }.trusted_types_for_script_requirements(scope), + unsafe { &*host_ptr }.allows_trusted_types_eval(scope), + Some(host_ptr), + ) + }; let action = if trusted_types_requirements.requires_conversion() { - if unsafe { &*host_ptr }.allows_trusted_types_eval(scope) { + if allow_trusted_types_eval { // The keyword relaxes Trusted Types conversion, but it does not // override another CSP policy. The per-policy CSP gate still runs. if source.is_string() { @@ -244,11 +258,20 @@ pub(super) fn string_code_generation_check_callback<'s>( source, modified_source: replacement, } => { - if !unsafe { &mut *host_ptr }.allows_eval_code_generation_by_csp( - scope, - allow_trusted_types_eval, - source.as_deref(), - ) { + let allowed = match host_ptr { + Some(host_ptr) => unsafe { &mut *host_ptr }.allows_eval_code_generation_by_csp( + scope, + allow_trusted_types_eval, + source.as_deref(), + ), + None => crate::worker::worker_allows_eval_code_generation_by_csp( + scope, + allow_trusted_types_eval, + source.as_deref(), + ) + .unwrap_or(false), + }; + if !allowed { return code_generation_result(false, None); } let replacement = match replacement { diff --git a/moli-renderer-v8/src/worker/global_scope/bootstrap.rs b/moli-renderer-v8/src/worker/global_scope/bootstrap.rs index 046c5b1c0d..2a205b070f 100644 --- a/moli-renderer-v8/src/worker/global_scope/bootstrap.rs +++ b/moli-renderer-v8/src/worker/global_scope/bootstrap.rs @@ -156,10 +156,18 @@ pub(in crate::worker) fn install_worker_global_scope<'s>( crate::content_security_policy::content_security_policy_requires_trusted_types_for_script( &state.borrow().content_security_policies, ); - if require_trusted_types_for_script { + // V8 bypasses the callback for primitive strings unless this fast path is disabled. + let has_string_code_generation_policy = { + let state = state.borrow(); + !state.content_security_policies.is_empty() + || !state.content_security_report_only_policies.is_empty() + }; + if has_string_code_generation_policy { scope .get_current_context() .set_allow_generation_from_strings(false); + } + if require_trusted_types_for_script { crate::context_bootstrap::install_trusted_types_eval_runtime_state(scope, global)?; } crate::context_bootstrap::install_webassembly_runtime_state(scope, global)?; diff --git a/moli-renderer-v8/src/worker/global_scope/content_security_policy.rs b/moli-renderer-v8/src/worker/global_scope/content_security_policy.rs index 7d7b322183..b16f889296 100644 --- a/moli-renderer-v8/src/worker/global_scope/content_security_policy.rs +++ b/moli-renderer-v8/src/worker/global_scope/content_security_policy.rs @@ -981,12 +981,19 @@ pub(crate) fn worker_allows_trusted_type_policy_name( ) } -pub(crate) fn worker_requires_trusted_types_for_script( +pub(crate) fn worker_trusted_types_for_script_requirements( scope: &mut v8::PinScope<'_, '_>, -) -> Option { +) -> Option { + let state = get_worker_state(scope)?; + let state = state.borrow(); Some( - crate::content_security_policy::content_security_policy_requires_trusted_types_for_script( - &get_worker_state(scope)?.borrow().content_security_policies, + crate::content_security_policy::TrustedTypesForScriptRequirements::new( + crate::content_security_policy::content_security_policy_requires_trusted_types_for_script( + &state.content_security_policies, + ), + crate::content_security_policy::content_security_policy_requires_trusted_types_for_script( + &state.content_security_report_only_policies, + ), ), ) } diff --git a/moli-renderer-v8/src/worker/global_scope/mod.rs b/moli-renderer-v8/src/worker/global_scope/mod.rs index 290e3833ad..4cc542c333 100644 --- a/moli-renderer-v8/src/worker/global_scope/mod.rs +++ b/moli-renderer-v8/src/worker/global_scope/mod.rs @@ -176,7 +176,7 @@ pub(super) use content_security_policy::{ pub(crate) use content_security_policy::{ dispatch_worker_trusted_types_sink_violation_event, worker_allows_eval_code_generation_by_csp, worker_allows_trusted_type_policy_name, worker_allows_trusted_types_eval, - worker_requires_trusted_types_for_script, + worker_trusted_types_for_script_requirements, }; use event_handlers::install_worker_global_event_handler_accessors; pub(super) use event_handlers::{WORKER_GLOBAL_LISTENERS_SLOT, service_worker_fetch_handler_type}; diff --git a/moli-renderer-v8/src/worker/mod.rs b/moli-renderer-v8/src/worker/mod.rs index 45986d4ac8..625e2642e5 100644 --- a/moli-renderer-v8/src/worker/mod.rs +++ b/moli-renderer-v8/src/worker/mod.rs @@ -35,16 +35,17 @@ pub(crate) use global_scope::{ register_worker_webcrypto_task, register_worker_websocket, reserve_nested_worker_context, send_worker_websocket_binary, send_worker_websocket_text, service_worker_runtime_identity, try_worker_xhr_abort_callback, try_worker_xhr_reschedule_timeout_after_timeout_change, - try_worker_xhr_send_callback, worker_allows_trusted_type_policy_name, - worker_allows_trusted_types_eval, worker_broadcast_channel_registry, - worker_broadcast_channel_storage_key, worker_broadcast_channel_wake_sender, - worker_broadcast_channel_wrapper, worker_current_script_url, worker_global_is_closed, - worker_global_origin, worker_message_port_registry, worker_message_port_wake_sender, - worker_message_port_wrapper, worker_notification_permission_state, - worker_opfs_directory_iterator_registry, worker_opfs_handle_registry, - worker_realm_secure_context_available, worker_requires_trusted_types_for_script, + try_worker_xhr_send_callback, worker_allows_eval_code_generation_by_csp, + worker_allows_trusted_type_policy_name, worker_allows_trusted_types_eval, + worker_broadcast_channel_registry, worker_broadcast_channel_storage_key, + worker_broadcast_channel_wake_sender, worker_broadcast_channel_wrapper, + worker_current_script_url, worker_global_is_closed, worker_global_origin, + worker_message_port_registry, worker_message_port_wake_sender, worker_message_port_wrapper, + worker_notification_permission_state, worker_opfs_directory_iterator_registry, + worker_opfs_handle_registry, worker_realm_secure_context_available, worker_service_worker_control_state, worker_storage_key, worker_storage_partition_identity, - worker_termination_requested, worker_uses_shared_worker_agent_cluster, + worker_termination_requested, worker_trusted_types_for_script_requirements, + worker_uses_shared_worker_agent_cluster, }; pub(crate) use handle::WorkerMessage; pub(crate) use handle::{ diff --git a/moli-renderer-v8/src/worker/thread/isolate.rs b/moli-renderer-v8/src/worker/thread/isolate.rs index 64e0324669..1c1893ad0f 100644 --- a/moli-renderer-v8/src/worker/thread/isolate.rs +++ b/moli-renderer-v8/src/worker/thread/isolate.rs @@ -43,7 +43,7 @@ impl WorkerIsolateState { worker_dynamic_import_with_phase_callback, ); isolate.set_modify_code_generation_from_strings_callback( - crate::context_bootstrap::trusted_types_code_generation_check_callback, + crate::script_vm::string_code_generation_check_callback, ); let runtime_inspector = WorkerRuntimeInspector::new( &mut isolate, diff --git a/moli-renderer-v8/src/worker/thread/tests/modules.rs b/moli-renderer-v8/src/worker/thread/tests/modules.rs index a2b4e63f2e..6902537302 100644 --- a/moli-renderer-v8/src/worker/thread/tests/modules.rs +++ b/moli-renderer-v8/src/worker/thread/tests/modules.rs @@ -810,6 +810,47 @@ async fn worker_trusted_types_policy_callbacks_follow_webidl_contract() { ); } +#[tokio::test] +async fn worker_csp_string_compilation_keywords_preserve_source_token_boundaries() { + ensure_v8(); + for keyword in ["unsafe-eval", "wasm-unsafe-eval", "none"] { + for (prefix, suffix, valid) in [ + ("", "", true), + ("\t\n", " \r\u{000c}", true), + ("\u{000b}", "", false), + ("", "\u{000b}", false), + ] { + let policy = format!("script-src {prefix}'{keyword}'{suffix}"); + let source = format!( + "postMessage(({}).slice(0, 2)); close();", + include_str!("../../../../tests/fixtures/csp-eval-source-tokens.js"), + ); + let options = + WorkerSpawnOptions::new(source, "https://app.test/worker/main.js".to_owned()) + .with_content_security_policies(vec![policy.clone()]); + let mut handle = spawn_test_worker_with_options(options); + let message = timeout(TIMEOUT, handle.recv()).await.unwrap().unwrap(); + let expected = serde_json::json!([ + if valid && keyword == "unsafe-eval" { + "allowed" + } else { + "EvalError" + }, + if valid && keyword == "unsafe-eval" { + "allowed" + } else { + "EvalError" + }, + ]); + assert_eq!( + expect_post_json(message), + expected.to_string(), + "{policy:?}" + ); + } + } +} + #[tokio::test] async fn worker_string_timer_csp_blocks_and_reports_without_blocking_functions() { ensure_v8();