diff --git a/moli-renderer-v8/src/native_bridge/document/attributes/attr_object/instance/accessors.rs b/moli-renderer-v8/src/native_bridge/document/attributes/attr_object/instance/accessors.rs index ad79f6b399..f942dc6c0c 100644 --- a/moli-renderer-v8/src/native_bridge/document/attributes/attr_object/instance/accessors.rs +++ b/moli-renderer-v8/src/native_bridge/document/attributes/attr_object/instance/accessors.rs @@ -2,8 +2,9 @@ use super::value::{attr_current_value, attr_owner_document_object, attr_owner_el use super::*; use crate::custom_elements; use crate::native_bridge::element::{ - set_live_element_attribute_appending_to_current_reaction_queue, + TrustedAttributeSetter, set_live_element_attribute_appending_to_current_reaction_queue, set_live_element_attribute_ns_appending_to_current_reaction_queue, + trusted_attribute_string_value, }; use crate::native_bridge::node_runtime_and_handle_from_object; use crate::webidl; @@ -152,30 +153,87 @@ pub(super) fn attr_instance_value_setter<'a>( return; } }; - let Some(state) = attr_state_object(scope, args.holder()) else { + let attr = args.holder(); + let Some(state) = attr_state_object(scope, attr) else { return; }; - let _ = state.set( - scope, - v8str(scope, "value").into(), - v8_string(scope, &string_value) - .map(Into::>::into) - .unwrap_or_else(|| v8::String::empty(scope).into()), - ); let Some(name) = object_string_property(scope, state, "name") else { return; }; - if let Some(owner) = object_property_as_object(scope, state, "ownerElement") { - if attr_value_setter_wrote_live_native(scope, owner, state, &name, &string_value) { - return; - } - if attr_value_setter_wrote_native(scope, owner, state, &name, &string_value) { - return; - } - let namespace = state - .get(scope, v8str(scope, "namespaceURI").into()) - .unwrap_or_else(|| v8::null(scope).into()); - if namespace.is_null_or_undefined() && name == name.to_ascii_lowercase() { + let Some(original_owner) = object_property_as_object(scope, state, "ownerElement") else { + set_attr_state_value(scope, state, &string_value); + return; + }; + let namespace = attr_state_nullable_string(scope, state, "namespaceURI"); + let local_name = object_string_property(scope, state, "localName") + .filter(|local_name| !local_name.is_empty()) + .or_else(|| name.rsplit_once(':').map(|(_, local)| local.to_owned())) + .unwrap_or_else(|| name.clone()); + let runtime_and_handle = node_runtime_and_handle_from_object(scope, original_owner) + .ok() + .or_else(|| detached_native_element_runtime_and_handle(scope, original_owner)); + let Some(verified_value) = trusted_attribute_string_value( + scope, + runtime_and_handle, + namespace.as_deref(), + &local_name, + &string_value, + TrustedAttributeSetter::AttrValue, + ) else { + return; + }; + + let Some(state) = attr_state_object(scope, attr) else { + return; + }; + let Some(owner) = object_property_as_object(scope, state, "ownerElement") else { + set_attr_state_value(scope, state, &verified_value); + return; + }; + if !owner.strict_equals(original_owner.into()) { + return; + } + + set_attr_state_value(scope, state, &verified_value); + if attr_value_setter_wrote_live_native(scope, owner, state, &name, &verified_value) { + return; + } + if attr_value_setter_wrote_native(scope, owner, state, &name, &verified_value) { + return; + } + let namespace_value = state + .get(scope, v8str(scope, "namespaceURI").into()) + .unwrap_or_else(|| v8::null(scope).into()); + if namespace_value.is_null_or_undefined() && name == name.to_ascii_lowercase() { + let _ = call_object_method( + scope, + owner, + "setAttribute", + &[ + v8_string(scope, &name) + .map(Into::>::into) + .unwrap_or_else(|| v8::String::empty(scope).into()), + v8_string(scope, &verified_value) + .map(Into::>::into) + .unwrap_or_else(|| v8::String::empty(scope).into()), + ], + ); + } else { + let set_attribute_ns_result = call_object_method( + scope, + owner, + "setAttributeNS", + &[ + namespace_value, + v8_string(scope, &name) + .map(Into::>::into) + .unwrap_or_else(|| v8::String::empty(scope).into()), + v8_string(scope, &verified_value) + .map(Into::>::into) + .unwrap_or_else(|| v8::String::empty(scope).into()), + ], + ); + if set_attribute_ns_result.is_none() { let _ = call_object_method( scope, owner, @@ -184,45 +242,29 @@ pub(super) fn attr_instance_value_setter<'a>( v8_string(scope, &name) .map(Into::>::into) .unwrap_or_else(|| v8::String::empty(scope).into()), - v8_string(scope, &string_value) + v8_string(scope, &verified_value) .map(Into::>::into) .unwrap_or_else(|| v8::String::empty(scope).into()), ], ); - } else { - let set_attribute_ns_result = call_object_method( - scope, - owner, - "setAttributeNS", - &[ - namespace, - v8_string(scope, &name) - .map(Into::>::into) - .unwrap_or_else(|| v8::String::empty(scope).into()), - v8_string(scope, &string_value) - .map(Into::>::into) - .unwrap_or_else(|| v8::String::empty(scope).into()), - ], - ); - if set_attribute_ns_result.is_none() { - let _ = call_object_method( - scope, - owner, - "setAttribute", - &[ - v8_string(scope, &name) - .map(Into::>::into) - .unwrap_or_else(|| v8::String::empty(scope).into()), - v8_string(scope, &string_value) - .map(Into::>::into) - .unwrap_or_else(|| v8::String::empty(scope).into()), - ], - ); - } } } } +fn set_attr_state_value( + scope: &mut v8::PinScope<'_, '_>, + state: v8::Local<'_, v8::Object>, + value: &str, +) { + let _ = state.set( + scope, + v8str(scope, "value").into(), + v8_string(scope, value) + .map(Into::>::into) + .unwrap_or_else(|| v8::String::empty(scope).into()), + ); +} + fn attr_value_setter_wrote_live_native<'s>( scope: &mut v8::PinScope<'s, '_>, owner: v8::Local<'s, v8::Object>, diff --git a/moli-renderer-v8/src/native_bridge/document/attributes/attribute_node.rs b/moli-renderer-v8/src/native_bridge/document/attributes/attribute_node.rs index fd41a981b5..9b196dbe00 100644 --- a/moli-renderer-v8/src/native_bridge/document/attributes/attribute_node.rs +++ b/moli-renderer-v8/src/native_bridge/document/attributes/attribute_node.rs @@ -2,10 +2,11 @@ use super::*; use crate::custom_elements; use crate::dom::native::Attribute; use crate::native_bridge::element::{ - remove_live_element_attribute_appending_to_current_reaction_queue, + TrustedAttributeSetter, remove_live_element_attribute_appending_to_current_reaction_queue, remove_live_element_attribute_ns_appending_to_current_reaction_queue, set_live_element_attribute_appending_to_current_reaction_queue, set_live_element_attribute_ns_appending_to_current_reaction_queue, + trusted_attribute_string_value, }; use crate::native_bridge::node_runtime_and_handle_from_object; use crate::webidl; @@ -129,17 +130,6 @@ pub(in crate::native_bridge::document) fn live_set_attribute_node<'s>( let Some(mut metadata) = detached_attr_node_metadata(scope, state) else { return Some(v8::null(scope).into()); }; - let owner = object_property_as_object(scope, state, "ownerElement"); - if owner.is_some_and(|owner| !owner.strict_equals(element.into())) { - throw_dom_exception( - scope, - "InUseAttributeError", - 10, - "The attribute is already in use by another element.", - ); - return Some(v8::undefined(scope).into()); - } - let Ok((runtime_ptr, handle)) = node_runtime_and_handle_from_object(scope, element) else { return Some(v8::null(scope).into()); }; @@ -155,6 +145,30 @@ pub(in crate::native_bridge::document) fn live_set_attribute_node<'s>( metadata.local_name = metadata.name.clone(); metadata.prefix = None; } + // DOM's "set an attribute" steps perform Trusted Types conversion before + // checking InUseAttributeError. The policy callback can mutate ownership, + // so the owner check below must observe its result. + let verified_value = trusted_attribute_string_value( + scope, + Some((runtime_ptr, handle)), + metadata.namespace_uri.as_deref(), + &metadata.local_name, + &metadata.value, + TrustedAttributeSetter::SetAttributeNode, + )?; + metadata.value = verified_value; + + let owner = object_property_as_object(scope, state, "ownerElement"); + if owner.is_some_and(|owner| !owner.strict_equals(element.into())) { + throw_dom_exception( + scope, + "InUseAttributeError", + 10, + "The attribute is already in use by another element.", + ); + return Some(v8::undefined(scope).into()); + } + let old_metadata = live_native_attribute_metadata_for_target(unsafe { &*runtime_ptr }, handle, &metadata); let old = old_metadata.as_ref().and_then(|old_metadata| { @@ -353,6 +367,24 @@ pub(in crate::native_bridge::document) fn detached_native_set_attribute_node<'s> let Some(mut metadata) = detached_attr_node_metadata(scope, state) else { return Some(v8::null(scope).into()); }; + let runtime_and_handle = detached_native_element_runtime_and_handle(scope, element); + if metadata.namespace_uri.is_none() { + metadata.name = detached_attribute_name(scope, element, &metadata.name); + metadata.local_name = metadata.name.clone(); + metadata.prefix = None; + } + // Keep this in the same order as the live path and the DOM algorithm: + // Trusted Types conversion precedes the InUseAttributeError owner check. + let verified_value = trusted_attribute_string_value( + scope, + runtime_and_handle, + metadata.namespace_uri.as_deref(), + &metadata.local_name, + &metadata.value, + TrustedAttributeSetter::SetAttributeNode, + )?; + metadata.value = verified_value; + let owner = object_property_as_object(scope, state, "ownerElement"); if owner.is_some_and(|owner| !owner.strict_equals(element.into())) { throw_dom_exception( @@ -372,8 +404,7 @@ pub(in crate::native_bridge::document) fn detached_native_set_attribute_node<'s> &metadata.local_name, ) } else { - let name = detached_attribute_name(scope, element, &metadata.name); - native_attr_object_by_name(scope, element, &name) + native_attr_object_by_name(scope, element, &metadata.name) }; if old.is_some_and(|old| old.strict_equals(attr.into())) { return Some(attr.into()); @@ -397,15 +428,13 @@ pub(in crate::native_bridge::document) fn detached_native_set_attribute_node<'s> ); cache_attached_attr_node(scope, element, attr, &metadata); } else { - let name = detached_attribute_name(scope, element, &metadata.name); - clear_live_attr_cache_entry(scope, element, &name); + clear_live_attr_cache_entry(scope, element, &metadata.name); let _ = write_detached_native_attribute_appending_to_current_reaction_queue( scope, element, - &name, + &metadata.name, &metadata.value, ); - metadata.name = name; cache_attached_attr_node(scope, element, attr, &metadata); } diff --git a/moli-renderer-v8/src/native_bridge/element.rs b/moli-renderer-v8/src/native_bridge/element.rs index 8f20acb3fe..6c709e0075 100644 --- a/moli-renderer-v8/src/native_bridge/element.rs +++ b/moli-renderer-v8/src/native_bridge/element.rs @@ -55,7 +55,7 @@ pub(crate) use script_execution::{ inline_script_source_for_execution, prepare_inline_classic_frame_script_job_for_execution, }; pub(in crate::native_bridge) use trusted_types::{ - TrustedAttributeSetter, trusted_attribute_value_string, + TrustedAttributeSetter, trusted_attribute_string_value, trusted_attribute_value_string, }; use trusted_types::{ TrustedScriptElementSink, trusted_script_element_sink_string, trusted_script_url_sink_string, diff --git a/moli-renderer-v8/src/native_bridge/element/trusted_types.rs b/moli-renderer-v8/src/native_bridge/element/trusted_types.rs index bc76d75763..7ef7d92c49 100644 --- a/moli-renderer-v8/src/native_bridge/element/trusted_types.rs +++ b/moli-renderer-v8/src/native_bridge/element/trusted_types.rs @@ -5,6 +5,8 @@ use crate::document_runtime::DomHandle; pub(in crate::native_bridge) enum TrustedAttributeSetter { SetAttribute, SetAttributeNs, + SetAttributeNode, + AttrValue, } impl TrustedAttributeSetter { @@ -12,6 +14,8 @@ impl TrustedAttributeSetter { match self { Self::SetAttribute => "setAttribute", Self::SetAttributeNs => "setAttributeNS", + Self::SetAttributeNode => "setAttributeNode", + Self::AttrValue => "value", } } @@ -19,6 +23,10 @@ impl TrustedAttributeSetter { match self { Self::SetAttribute => crate::webidl::Context::argument("Element setAttribute", 2), Self::SetAttributeNs => crate::webidl::Context::argument("Element setAttributeNS", 3), + Self::SetAttributeNode => { + crate::webidl::Context::argument("Element setAttributeNode", 1) + } + Self::AttrValue => crate::webidl::Context::member("Attr", "value"), } } } @@ -249,6 +257,25 @@ pub(in crate::native_bridge) fn trusted_attribute_value_string<'s>( } } +pub(in crate::native_bridge) fn trusted_attribute_string_value<'s>( + scope: &mut v8::PinScope<'s, '_>, + runtime_and_handle: Option<(*mut JsContextHost, DomHandle)>, + attribute_namespace: Option<&str>, + local_name: &str, + value: &str, + setter: TrustedAttributeSetter, +) -> Option { + let value = crate::util::v8_string(scope, value)?; + trusted_attribute_value_string( + scope, + runtime_and_handle, + attribute_namespace, + local_name, + value.into(), + setter, + ) +} + pub(crate) fn trusted_script_source_for_execution( scope: &mut v8::PinScope<'_, '_>, runtime_ptr: *mut JsContextHost, diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/trusted_types.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/trusted_types.rs index b5217eac41..1820fffc89 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/trusted_types.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/trusted_types.rs @@ -1055,6 +1055,115 @@ fn url_attribute_writes_enforce_the_non_iframe_trusted_script_url_sink_table() { ); } +#[test] +fn attached_attribute_mutations_recheck_trusted_types_after_domstring_conversion() { + let mut vm = new_storage_test_vm("https://attached-attribute-trusted-types.test/"); + vm.set_response_content_security_policies(&["require-trusted-types-for 'script'".to_owned()]); + + let result = vm + .eval( + r#" +(() => { + const explicit = trustedTypes.createPolicy("attached-attribute", { + createScript: value => value + }); + + const alreadyAttached = document.createElement("button"); + alreadyAttached.setAttribute("onclick", explicit.createScript("same-input")); + const alreadyAttachedAttr = alreadyAttached.getAttributeNode("onclick"); + let sameAttributeRejected = false; + try { + alreadyAttached.setAttributeNode(alreadyAttachedAttr); + } catch (error) { + sameAttributeRejected = error instanceof TypeError; + } + + const calls = []; + let detachAttr; + let moveAttr; + let moveTarget; + let moving = false; + trustedTypes.createPolicy("default", { + createScript: (value, type, sink) => { + calls.push([value, type, sink]); + if (value === "detach-input") { + detachAttr.ownerElement.removeAttributeNode(detachAttr); + } + if (value === "move-input" && !moving) { + moving = true; + const owner = moveAttr.ownerElement; + if (owner) owner.removeAttributeNode(moveAttr); + moveTarget.setAttributeNode(moveAttr); + moving = false; + } + return `safe-${value}`; + } + }); + + const nodeSetters = [ + ["setAttributeNode", (element, attr) => element.setAttributeNode(attr)], + ["setAttributeNodeNS", (element, attr) => element.setAttributeNodeNS(attr)], + ["setNamedItem", (element, attr) => element.attributes.setNamedItem(attr)], + ["setNamedItemNS", (element, attr) => element.attributes.setNamedItemNS(attr)] + ]; + const nodeValues = nodeSetters.map(([name, setter], index) => { + const element = document.createElement("button"); + const attr = document.createAttribute("onclick"); + attr.value = index === 0 + ? explicit.createScript(`${name}-input`) + : `${name}-input`; + const previous = setter(element, attr); + return [previous === null, attr.ownerElement === element, attr.value]; + }); + + const valueSetters = [ + ["value", (attr, value) => { attr.value = value; }], + ["nodeValue", (attr, value) => { attr.nodeValue = value; }], + ["textContent", (attr, value) => { attr.textContent = value; }] + ]; + const valueResults = valueSetters.map(([name, setter], index) => { + const element = document.createElement("button"); + element.setAttribute("onclick", explicit.createScript("initial")); + const attr = element.getAttributeNode("onclick"); + setter(attr, index === 0 ? explicit.createScript(`${name}-input`) : `${name}-input`); + return [attr.ownerElement === element, attr.value]; + }); + + const detachedOwner = document.createElement("button"); + detachedOwner.setAttribute("onclick", explicit.createScript("initial")); + detachAttr = detachedOwner.getAttributeNode("onclick"); + detachAttr.value = "detach-input"; + + const originalMoveOwner = document.createElement("button"); + moveTarget = document.createElement("button"); + moveAttr = document.createAttribute("onclick"); + moveAttr.value = "move-input"; + let moveRejected = false; + try { + originalMoveOwner.setAttributeNode(moveAttr); + } catch (error) { + moveRejected = error.name === "InUseAttributeError"; + } + + return JSON.stringify({ + sameAttributeRejected, + nodeValues, + valueResults, + detached: [detachAttr.ownerElement === null, detachAttr.value], + moved: [moveRejected, moveAttr.ownerElement === moveTarget, moveAttr.value], + calls + }); +})() +"#, + ) + .expect("attached attribute Trusted Types mutation probe should evaluate"); + + assert_eq!( + result, + r#"{"sameAttributeRejected":true,"nodeValues":[[true,true,"safe-setAttributeNode-input"],[true,true,"safe-setAttributeNodeNS-input"],[true,true,"safe-setNamedItem-input"],[true,true,"safe-setNamedItemNS-input"]],"valueResults":[[true,"safe-value-input"],[true,"safe-nodeValue-input"],[true,"safe-textContent-input"]],"detached":[true,"safe-detach-input"],"moved":[true,true,"safe-move-input"],"calls":[["setAttributeNode-input","TrustedScript","Element onclick"],["setAttributeNodeNS-input","TrustedScript","Element onclick"],["setNamedItem-input","TrustedScript","Element onclick"],["setNamedItemNS-input","TrustedScript","Element onclick"],["value-input","TrustedScript","Element onclick"],["nodeValue-input","TrustedScript","Element onclick"],["textContent-input","TrustedScript","Element onclick"],["detach-input","TrustedScript","Element onclick"],["move-input","TrustedScript","Element onclick"],["move-input","TrustedScript","Element onclick"]]}"# + ); +} + #[test] fn empty_default_policy_reports_each_rejected_element_sink() { let mut vm = new_storage_test_vm("https://empty-default-policy.test/");