mirror of
https://github.com/servo/servo.git
synced 2025-07-22 23:03:42 +01:00
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 <tvanderlippe@gmail.com>
This commit is contained in:
parent
565e16178f
commit
d0de4e64d2
10 changed files with 124 additions and 79 deletions
|
@ -12,6 +12,7 @@ use std::rc::Rc;
|
||||||
use std::str::FromStr;
|
use std::str::FromStr;
|
||||||
use std::{fmt, mem};
|
use std::{fmt, mem};
|
||||||
|
|
||||||
|
use content_security_policy as csp;
|
||||||
use cssparser::match_ignore_ascii_case;
|
use cssparser::match_ignore_ascii_case;
|
||||||
use devtools_traits::AttrInfo;
|
use devtools_traits::AttrInfo;
|
||||||
use dom_struct::dom_struct;
|
use dom_struct::dom_struct;
|
||||||
|
@ -2120,6 +2121,59 @@ impl Element {
|
||||||
node.owner_doc().element_attr_will_change(self, attr);
|
node.owner_doc().element_attr_will_change(self, attr);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <https://html.spec.whatwg.org/multipage/#the-style-attribute>
|
||||||
|
fn update_style_attribute(&self, attr: &Attr, mutation: AttributeMutation) {
|
||||||
|
let doc = self.upcast::<Node>().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
|
// https://dom.spec.whatwg.org/#insert-adjacent
|
||||||
pub(crate) fn insert_adjacent(
|
pub(crate) fn insert_adjacent(
|
||||||
&self,
|
&self,
|
||||||
|
@ -3800,43 +3854,7 @@ impl VirtualMethods for Element {
|
||||||
&local_name!("tabindex") | &local_name!("draggable") | &local_name!("hidden") => {
|
&local_name!("tabindex") | &local_name!("draggable") | &local_name!("hidden") => {
|
||||||
self.update_sequentially_focusable_status(can_gc)
|
self.update_sequentially_focusable_status(can_gc)
|
||||||
},
|
},
|
||||||
&local_name!("style") => {
|
&local_name!("style") => self.update_style_attribute(attr, mutation),
|
||||||
// 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!("id") => {
|
&local_name!("id") => {
|
||||||
*self.id_attribute.borrow_mut() = mutation.new_value(attr).and_then(|value| {
|
*self.id_attribute.borrow_mut() = mutation.new_value(attr).and_then(|value| {
|
||||||
let value = value.as_atom();
|
let value = value.as_atom();
|
||||||
|
|
7
tests/wpt/meta/MANIFEST.json
vendored
7
tests/wpt/meta/MANIFEST.json
vendored
|
@ -571866,6 +571866,13 @@
|
||||||
{}
|
{}
|
||||||
]
|
]
|
||||||
],
|
],
|
||||||
|
"style-src-inline-style-with-csstext.html": [
|
||||||
|
"5e812b4aee9d0d081673a0f333f8b29187619c3d",
|
||||||
|
[
|
||||||
|
null,
|
||||||
|
{}
|
||||||
|
]
|
||||||
|
],
|
||||||
"style-src-multiple-policies-multiple-hashing-algorithms.html": [
|
"style-src-multiple-policies-multiple-hashing-algorithms.html": [
|
||||||
"027c61d8c632f2387408b8fb6869dee69bb8913d",
|
"027c61d8c632f2387408b8fb6869dee69bb8913d",
|
||||||
[
|
[
|
||||||
|
|
|
@ -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
|
|
|
@ -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
|
|
|
@ -1,13 +1,36 @@
|
||||||
[inline-style-allowed-while-cloning-objects.sub.html]
|
[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]
|
[non-HTML namespace]
|
||||||
expected: FAIL
|
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
|
||||||
|
|
|
@ -1,3 +0,0 @@
|
||||||
[inline-style-attribute-blocked.sub.html]
|
|
||||||
[Expecting logs: ["violated-directive=style-src-attr","PASS"\]]
|
|
||||||
expected: FAIL
|
|
|
@ -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
|
|
|
@ -1,4 +0,0 @@
|
||||||
[style_attribute_denied_missing_unsafe_hashes.html]
|
|
||||||
expected: TIMEOUT
|
|
||||||
[Test that the inline style attribute is blocked]
|
|
||||||
expected: NOTRUN
|
|
|
@ -1,4 +0,0 @@
|
||||||
[style_attribute_denied_wrong_hash.html]
|
|
||||||
expected: TIMEOUT
|
|
||||||
[Test that the inline style attribute is blocked]
|
|
||||||
expected: NOTRUN
|
|
29
tests/wpt/tests/content-security-policy/style-src/style-src-inline-style-with-csstext.html
vendored
Normal file
29
tests/wpt/tests/content-security-policy/style-src/style-src-inline-style-with-csstext.html
vendored
Normal file
|
@ -0,0 +1,29 @@
|
||||||
|
<!doctype html>
|
||||||
|
<html>
|
||||||
|
<head>
|
||||||
|
<meta http-equiv="Content-Security-Policy" content="style-src 'self';">
|
||||||
|
<script src="/resources/testharness.js"></script>
|
||||||
|
<script src="/resources/testharnessreport.js"></script>
|
||||||
|
|
||||||
|
<script>
|
||||||
|
var t = async_test("Manipulating cssText should be allowed with 'self'");
|
||||||
|
document.addEventListener("securitypolicyviolation", t.unreached_func("Should not trigger a security policy violation"));
|
||||||
|
</script>
|
||||||
|
</head>
|
||||||
|
<body>
|
||||||
|
<div id='log'></div>
|
||||||
|
|
||||||
|
<div id="content">Lorem ipsum</div>
|
||||||
|
|
||||||
|
<script>
|
||||||
|
t.step(function() {
|
||||||
|
var contentEl = document.getElementById("content");
|
||||||
|
contentEl.style.cssText = 'margin-left: 2px;';
|
||||||
|
var marginLeftVal = getComputedStyle(contentEl).getPropertyValue('margin-left');
|
||||||
|
assert_equals(marginLeftVal, "2px");
|
||||||
|
t.done();
|
||||||
|
});
|
||||||
|
</script>
|
||||||
|
|
||||||
|
</body>
|
||||||
|
</html>
|
Loading…
Add table
Add a link
Reference in a new issue