From d0de4e64d27fa029714b9f9e080491d0fec8c243 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Fri, 9 May 2025 19:36:55 +0200 Subject: [PATCH] Add CSP check for inline style attribute (#36923) To be able to abort the update, extract the functionality into a separate method. Otherwise, we don't run the `node.rev_version` at the end, which according to the comment is probably important. Not all `style-src` tests pass and I don't fully understand why yet, but I presume it has to do with some special quirks of stylesheets that other CSP checks don't have. All `style-src-attr-elem` tests pass though. Part of #4577 Signed-off-by: Tim van der Lippe --- components/script/dom/element.rs | 92 +++++++++++-------- tests/wpt/meta/MANIFEST.json | 7 ++ ...tyle-src-attr-blocked-src-allowed.html.ini | 7 -- ...yle-src-elem-allowed-attr-blocked.html.ini | 7 -- ...allowed-while-cloning-objects.sub.html.ini | 43 +++++++-- ...nline-style-attribute-blocked.sub.html.ini | 3 - ...rc-inline-style-attribute-blocked.html.ini | 7 -- ...bute_denied_missing_unsafe_hashes.html.ini | 4 - ...style_attribute_denied_wrong_hash.html.ini | 4 - .../style-src-inline-style-with-csstext.html | 29 ++++++ 10 files changed, 124 insertions(+), 79 deletions(-) delete mode 100644 tests/wpt/meta/content-security-policy/style-src-attr-elem/style-src-attr-blocked-src-allowed.html.ini delete mode 100644 tests/wpt/meta/content-security-policy/style-src-attr-elem/style-src-elem-allowed-attr-blocked.html.ini delete mode 100644 tests/wpt/meta/content-security-policy/style-src/inline-style-attribute-blocked.sub.html.ini delete mode 100644 tests/wpt/meta/content-security-policy/style-src/style-src-inline-style-attribute-blocked.html.ini delete mode 100644 tests/wpt/meta/content-security-policy/unsafe-hashes/style_attribute_denied_missing_unsafe_hashes.html.ini delete mode 100644 tests/wpt/meta/content-security-policy/unsafe-hashes/style_attribute_denied_wrong_hash.html.ini create mode 100644 tests/wpt/tests/content-security-policy/style-src/style-src-inline-style-with-csstext.html diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 7f38e55fb14..7770d0c8fa5 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -12,6 +12,7 @@ use std::rc::Rc; use std::str::FromStr; use std::{fmt, mem}; +use content_security_policy as csp; use cssparser::match_ignore_ascii_case; use devtools_traits::AttrInfo; use dom_struct::dom_struct; @@ -2120,6 +2121,59 @@ impl Element { node.owner_doc().element_attr_will_change(self, attr); } + /// + fn update_style_attribute(&self, attr: &Attr, mutation: AttributeMutation) { + let doc = self.upcast::().owner_doc(); + // Modifying the `style` attribute might change style. + *self.style_attribute.borrow_mut() = match mutation { + AttributeMutation::Set(..) => { + // This is the fast path we use from + // CSSStyleDeclaration. + // + // Juggle a bit to keep the borrow checker happy + // while avoiding the extra clone. + let is_declaration = matches!(*attr.value(), AttrValue::Declaration(..)); + + let block = if is_declaration { + let mut value = AttrValue::String(String::new()); + attr.swap_value(&mut value); + let (serialization, block) = match value { + AttrValue::Declaration(s, b) => (s, b), + _ => unreachable!(), + }; + let mut value = AttrValue::String(serialization); + attr.swap_value(&mut value); + block + } else { + let win = self.owner_window(); + let source = &**attr.value(); + // However, if the Should element's inline behavior be blocked by + // Content Security Policy? algorithm returns "Blocked" when executed + // upon the attribute's element, "style attribute", and the attribute's value, + // then the style rules defined in the attribute's value must not be applied to the element. [CSP] + if doc.should_elements_inline_type_behavior_be_blocked( + self, + csp::InlineCheckType::StyleAttribute, + source, + ) == csp::CheckResult::Blocked + { + return; + } + Arc::new(doc.style_shared_lock().wrap(parse_style_attribute( + source, + &UrlExtraData(doc.base_url().get_arc()), + win.css_error_reporter(), + doc.quirks_mode(), + CssRuleType::Style, + ))) + }; + + Some(block) + }, + AttributeMutation::Removed => None, + }; + } + // https://dom.spec.whatwg.org/#insert-adjacent pub(crate) fn insert_adjacent( &self, @@ -3800,43 +3854,7 @@ impl VirtualMethods for Element { &local_name!("tabindex") | &local_name!("draggable") | &local_name!("hidden") => { self.update_sequentially_focusable_status(can_gc) }, - &local_name!("style") => { - // Modifying the `style` attribute might change style. - *self.style_attribute.borrow_mut() = match mutation { - AttributeMutation::Set(..) => { - // This is the fast path we use from - // CSSStyleDeclaration. - // - // Juggle a bit to keep the borrow checker happy - // while avoiding the extra clone. - let is_declaration = matches!(*attr.value(), AttrValue::Declaration(..)); - - let block = if is_declaration { - let mut value = AttrValue::String(String::new()); - attr.swap_value(&mut value); - let (serialization, block) = match value { - AttrValue::Declaration(s, b) => (s, b), - _ => unreachable!(), - }; - let mut value = AttrValue::String(serialization); - attr.swap_value(&mut value); - block - } else { - let win = self.owner_window(); - Arc::new(doc.style_shared_lock().wrap(parse_style_attribute( - &attr.value(), - &UrlExtraData(doc.base_url().get_arc()), - win.css_error_reporter(), - doc.quirks_mode(), - CssRuleType::Style, - ))) - }; - - Some(block) - }, - AttributeMutation::Removed => None, - }; - }, + &local_name!("style") => self.update_style_attribute(attr, mutation), &local_name!("id") => { *self.id_attribute.borrow_mut() = mutation.new_value(attr).and_then(|value| { let value = value.as_atom(); diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index 893b07e9e3f..e62e766680d 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -571866,6 +571866,13 @@ {} ] ], + "style-src-inline-style-with-csstext.html": [ + "5e812b4aee9d0d081673a0f333f8b29187619c3d", + [ + null, + {} + ] + ], "style-src-multiple-policies-multiple-hashing-algorithms.html": [ "027c61d8c632f2387408b8fb6869dee69bb8913d", [ diff --git a/tests/wpt/meta/content-security-policy/style-src-attr-elem/style-src-attr-blocked-src-allowed.html.ini b/tests/wpt/meta/content-security-policy/style-src-attr-elem/style-src-attr-blocked-src-allowed.html.ini deleted file mode 100644 index a5cf5faf238..00000000000 --- a/tests/wpt/meta/content-security-policy/style-src-attr-elem/style-src-attr-blocked-src-allowed.html.ini +++ /dev/null @@ -1,7 +0,0 @@ -[style-src-attr-blocked-src-allowed.html] - expected: TIMEOUT - [Should fire a security policy violation event] - expected: NOTRUN - - [The attribute style should not be applied] - expected: FAIL diff --git a/tests/wpt/meta/content-security-policy/style-src-attr-elem/style-src-elem-allowed-attr-blocked.html.ini b/tests/wpt/meta/content-security-policy/style-src-attr-elem/style-src-elem-allowed-attr-blocked.html.ini deleted file mode 100644 index 979fc151f38..00000000000 --- a/tests/wpt/meta/content-security-policy/style-src-attr-elem/style-src-elem-allowed-attr-blocked.html.ini +++ /dev/null @@ -1,7 +0,0 @@ -[style-src-elem-allowed-attr-blocked.html] - expected: TIMEOUT - [Should fire a security policy violation for the attribute] - expected: NOTRUN - - [The attribute style should not be applied and the inline style should be applied] - expected: FAIL diff --git a/tests/wpt/meta/content-security-policy/style-src/inline-style-allowed-while-cloning-objects.sub.html.ini b/tests/wpt/meta/content-security-policy/style-src/inline-style-allowed-while-cloning-objects.sub.html.ini index 0c8111987c0..c99d7bd7844 100644 --- a/tests/wpt/meta/content-security-policy/style-src/inline-style-allowed-while-cloning-objects.sub.html.ini +++ b/tests/wpt/meta/content-security-policy/style-src/inline-style-allowed-while-cloning-objects.sub.html.ini @@ -1,13 +1,36 @@ [inline-style-allowed-while-cloning-objects.sub.html] - expected: TIMEOUT - [Test that violation report event was fired] - expected: NOTRUN - - [inline-style-allowed-while-cloning-objects 12] - expected: FAIL - - [inline-style-allowed-while-cloning-objects 14] - expected: FAIL - [non-HTML namespace] expected: FAIL + + [inline-style-allowed-while-cloning-objects 1] + expected: FAIL + + [inline-style-allowed-while-cloning-objects 3] + expected: FAIL + + [inline-style-allowed-while-cloning-objects 5] + expected: FAIL + + [inline-style-allowed-while-cloning-objects 7] + expected: FAIL + + [inline-style-allowed-while-cloning-objects 8] + expected: FAIL + + [inline-style-allowed-while-cloning-objects 9] + expected: FAIL + + [inline-style-allowed-while-cloning-objects 10] + expected: FAIL + + [inline-style-allowed-while-cloning-objects 11] + expected: FAIL + + [inline-style-allowed-while-cloning-objects 17] + expected: FAIL + + [inline-style-allowed-while-cloning-objects 18] + expected: FAIL + + [inline-style-allowed-while-cloning-objects 19] + expected: FAIL diff --git a/tests/wpt/meta/content-security-policy/style-src/inline-style-attribute-blocked.sub.html.ini b/tests/wpt/meta/content-security-policy/style-src/inline-style-attribute-blocked.sub.html.ini deleted file mode 100644 index 92f00acdffe..00000000000 --- a/tests/wpt/meta/content-security-policy/style-src/inline-style-attribute-blocked.sub.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[inline-style-attribute-blocked.sub.html] - [Expecting logs: ["violated-directive=style-src-attr","PASS"\]] - expected: FAIL diff --git a/tests/wpt/meta/content-security-policy/style-src/style-src-inline-style-attribute-blocked.html.ini b/tests/wpt/meta/content-security-policy/style-src/style-src-inline-style-attribute-blocked.html.ini deleted file mode 100644 index d910f28e56a..00000000000 --- a/tests/wpt/meta/content-security-policy/style-src/style-src-inline-style-attribute-blocked.html.ini +++ /dev/null @@ -1,7 +0,0 @@ -[style-src-inline-style-attribute-blocked.html] - expected: TIMEOUT - [Inline style attribute should not be applied without 'unsafe-inline'] - expected: FAIL - - [Should fire a securitypolicyviolation event] - expected: NOTRUN diff --git a/tests/wpt/meta/content-security-policy/unsafe-hashes/style_attribute_denied_missing_unsafe_hashes.html.ini b/tests/wpt/meta/content-security-policy/unsafe-hashes/style_attribute_denied_missing_unsafe_hashes.html.ini deleted file mode 100644 index 26dc98e8f62..00000000000 --- a/tests/wpt/meta/content-security-policy/unsafe-hashes/style_attribute_denied_missing_unsafe_hashes.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[style_attribute_denied_missing_unsafe_hashes.html] - expected: TIMEOUT - [Test that the inline style attribute is blocked] - expected: NOTRUN diff --git a/tests/wpt/meta/content-security-policy/unsafe-hashes/style_attribute_denied_wrong_hash.html.ini b/tests/wpt/meta/content-security-policy/unsafe-hashes/style_attribute_denied_wrong_hash.html.ini deleted file mode 100644 index 3031a4f6f77..00000000000 --- a/tests/wpt/meta/content-security-policy/unsafe-hashes/style_attribute_denied_wrong_hash.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[style_attribute_denied_wrong_hash.html] - expected: TIMEOUT - [Test that the inline style attribute is blocked] - expected: NOTRUN diff --git a/tests/wpt/tests/content-security-policy/style-src/style-src-inline-style-with-csstext.html b/tests/wpt/tests/content-security-policy/style-src/style-src-inline-style-with-csstext.html new file mode 100644 index 00000000000..5e812b4aee9 --- /dev/null +++ b/tests/wpt/tests/content-security-policy/style-src/style-src-inline-style-with-csstext.html @@ -0,0 +1,29 @@ + + + + + + + + + + +
+ +
Lorem ipsum
+ + + + +