From 4f74ef81d1225829d92440bd7da6861045a4738a Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Mon, 7 Apr 2025 20:38:46 -0500 Subject: [PATCH] Fix escaped text in ssr stylesheets and scripts (#3933) --- packages/document/src/elements/meta.rs | 6 + packages/playwright-tests/fullstack.spec.js | 2 +- packages/ssr/src/cache.rs | 120 +++++++++- packages/ssr/src/renderer.rs | 123 ++++++---- packages/ssr/tests/escape.rs | 249 ++++++++++++++++++++ 5 files changed, 443 insertions(+), 57 deletions(-) create mode 100644 packages/ssr/tests/escape.rs diff --git a/packages/document/src/elements/meta.rs b/packages/document/src/elements/meta.rs index f79965353..01f08a0b3 100644 --- a/packages/document/src/elements/meta.rs +++ b/packages/document/src/elements/meta.rs @@ -69,6 +69,12 @@ pub fn Meta(props: MetaProps) -> Element { use_hook(|| { let document = document(); + let insert_link = document.create_head_component(); + + if !insert_link { + return; + } + document.create_meta(props); }); diff --git a/packages/playwright-tests/fullstack.spec.js b/packages/playwright-tests/fullstack.spec.js index 7732b98a0..07d3468fa 100644 --- a/packages/playwright-tests/fullstack.spec.js +++ b/packages/playwright-tests/fullstack.spec.js @@ -39,7 +39,7 @@ test("hydration", async ({ page }) => { }); test("document elements", async ({ page }) => { - await page.goto("http://localhost:9999"); + await page.goto("http://localhost:3333"); // wait until the meta element is mounted const meta = page.locator("meta#meta-head[name='testing']"); await meta.waitFor({ state: "attached" }); diff --git a/packages/ssr/src/cache.rs b/packages/ssr/src/cache.rs index 7790a278d..40cef3af6 100644 --- a/packages/ssr/src/cache.rs +++ b/packages/ssr/src/cache.rs @@ -78,14 +78,52 @@ impl AddAssign for StringChain { } } -#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +/// The escape text enum is used to mark segments that should be escaped +/// when rendering. This is used to prevent XSS attacks by escaping user input. +pub(crate) enum EscapeText { + /// Always escape the text. This will be assigned if the text node is under + /// a normal tag like a div in the template + Escape, + /// Don't escape the text. This will be assigned if the text node is under + /// a script or style tag in the template + NoEscape, + /// Only escape the tag if this is rendered under a script or style tag in + /// the parent template. This will be assigned if the text node is a root + /// node in the template + ParentEscape, +} + +impl EscapeText { + /// Check if the text should be escaped based on the parent's resolved + /// escape text value + pub fn should_escape(&self, parent_escaped: bool) -> bool { + match self { + EscapeText::Escape => true, + EscapeText::NoEscape => false, + EscapeText::ParentEscape => parent_escaped, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub(crate) enum Segment { /// A marker for where to insert an attribute with a given index Attr(usize), /// A marker for where to insert a node with a given index - Node(usize), + Node { + index: usize, + escape_text: EscapeText, + }, /// Text that we know is static in the template that is pre-rendered PreRendered(String), + /// Text we know is static in the template that is pre-rendered that may or may not be escaped + PreRenderedMaybeEscaped { + /// The text to render + value: String, + /// Only render this text if the escaped value is this + renderer_if_escaped: bool, + }, /// Anything between this and the segments at the index is only required for hydration. If you don't need to hydrate, you can safely skip to the section at the given index HydrationOnlySection(usize), /// A marker for where to insert a dynamic styles @@ -127,7 +165,14 @@ impl StringCache { let mut cur_path = vec![]; for (root_idx, root) in template.template.roots.iter().enumerate() { - from_template_recursive(root, &mut cur_path, root_idx, true, &mut chain)?; + from_template_recursive( + root, + &mut cur_path, + root_idx, + true, + EscapeText::ParentEscape, + &mut chain, + )?; } Ok(Self { @@ -141,6 +186,7 @@ fn from_template_recursive( cur_path: &mut Vec, root_idx: usize, is_root: bool, + escape_text: EscapeText, chain: &mut StringChain, ) -> Result<(), std::fmt::Error> { match root { @@ -171,10 +217,18 @@ fn from_template_recursive( styles.push((name, value)); } else if BOOL_ATTRS.contains(name) { if str_truthy(value) { - write!(chain, " {name}=\"{value}\"",)?; + write!( + chain, + " {name}=\"{}\"", + askama_escape::escape(value, askama_escape::Html) + )?; } } else { - write!(chain, " {name}=\"{value}\"")?; + write!( + chain, + " {name}=\"{}\"", + askama_escape::escape(value, askama_escape::Html) + )?; } } TemplateAttribute::Dynamic { id: index } => { @@ -189,7 +243,11 @@ fn from_template_recursive( if !styles.is_empty() { write!(chain, " style=\"")?; for (name, value) in styles { - write!(chain, "{name}:{value};")?; + write!( + chain, + "{name}:{};", + askama_escape::escape(value, askama_escape::Html) + )?; } *chain += Segment::StyleMarker { inside_style_tag: true, @@ -226,8 +284,15 @@ fn from_template_recursive( *chain += Segment::InnerHtmlMarker; } + // Escape the text in children if this is not a style or script tag. If it is a style + // or script tag, we want to allow the user to write code inside the tag + let escape_text = match *tag { + "style" | "script" => EscapeText::NoEscape, + _ => EscapeText::Escape, + }; + for child in *children { - from_template_recursive(child, cur_path, root_idx, false, chain)?; + from_template_recursive(child, cur_path, root_idx, false, escape_text, chain)?; } write!(chain, "")?; } @@ -243,16 +308,45 @@ fn from_template_recursive( std::fmt::Result::Ok(()) })?; } - write!( - chain, - "{}", - askama_escape::escape(text, askama_escape::Html) - )?; + match escape_text { + // If we know this is statically escaped we can just write it out + // rsx! { div { "hello" } } + EscapeText::Escape => { + write!( + chain, + "{}", + askama_escape::escape(text, askama_escape::Html) + )?; + } + // If we know this is statically not escaped we can just write it out + // rsx! { script { "console.log('hello')" } } + EscapeText::NoEscape => { + write!(chain, "{}", text)?; + } + // Otherwise, write out both versions and let the renderer decide which one to use + // at runtime + // rsx! { "console.log('hello')" } + EscapeText::ParentEscape => { + *chain += Segment::PreRenderedMaybeEscaped { + value: text.to_string(), + renderer_if_escaped: false, + }; + *chain += Segment::PreRenderedMaybeEscaped { + value: askama_escape::escape(text, askama_escape::Html).to_string(), + renderer_if_escaped: true, + }; + } + } if is_root { chain.if_hydration_enabled(|chain| write!(chain, ""))?; } } - TemplateNode::Dynamic { id: idx } => *chain += Segment::Node(*idx), + TemplateNode::Dynamic { id: idx } => { + *chain += Segment::Node { + index: *idx, + escape_text, + } + } } Ok(()) diff --git a/packages/ssr/src/renderer.rs b/packages/ssr/src/renderer.rs index d9e178f32..532743cae 100644 --- a/packages/ssr/src/renderer.rs +++ b/packages/ssr/src/renderer.rs @@ -95,7 +95,7 @@ impl Renderer { scope: ScopeId, ) -> std::fmt::Result { let node = dom.get_scope(scope).unwrap().root_node(); - self.render_template(buf, dom, node)?; + self.render_template(buf, dom, node, true)?; Ok(()) } @@ -105,6 +105,7 @@ impl Renderer { mut buf: &mut W, dom: &VirtualDom, template: &VNode, + parent_escaped: bool, ) -> std::fmt::Result { let entry = self .template_cache @@ -158,50 +159,66 @@ impl Renderer { } } } - Segment::Node(idx) => match &template.dynamic_nodes[*idx] { - DynamicNode::Component(node) => { - if let Some(render_components) = self.render_components.clone() { - let scope_id = node.mounted_scope_id(*idx, template, dom).unwrap(); + Segment::Node { index, escape_text } => { + let escaped = escape_text.should_escape(parent_escaped); + match &template.dynamic_nodes[*index] { + DynamicNode::Component(node) => { + if let Some(render_components) = self.render_components.clone() { + let scope_id = + node.mounted_scope_id(*index, template, dom).unwrap(); - render_components(self, &mut buf, dom, scope_id)?; - } else { - let scope = node.mounted_scope(*idx, template, dom).unwrap(); - let node = scope.root_node(); - self.render_template(buf, dom, node)? + render_components(self, &mut buf, dom, scope_id)?; + } else { + let scope = node.mounted_scope(*index, template, dom).unwrap(); + let node = scope.root_node(); + self.render_template(buf, dom, node, escaped)? + } } - } - DynamicNode::Text(text) => { - // in SSR, we are concerned that we can't hunt down the right text node since they might get merged - if self.pre_render { - write!(buf, "", self.dynamic_node_id)?; - self.dynamic_node_id += 1; + DynamicNode::Text(text) => { + // in SSR, we are concerned that we can't hunt down the right text node since they might get merged + if self.pre_render { + write!(buf, "", self.dynamic_node_id)?; + self.dynamic_node_id += 1; + } + + if escaped { + write!( + buf, + "{}", + askama_escape::escape(&text.value, askama_escape::Html) + )?; + } else { + write!(buf, "{}", text.value)?; + } + + if self.pre_render { + write!(buf, "")?; + } + } + DynamicNode::Fragment(nodes) => { + for child in nodes { + self.render_template(buf, dom, child, escaped)?; + } } - write!( - buf, - "{}", - askama_escape::escape(&text.value, askama_escape::Html) - )?; - - if self.pre_render { - write!(buf, "")?; + DynamicNode::Placeholder(_) => { + if self.pre_render { + write!(buf, "", self.dynamic_node_id)?; + self.dynamic_node_id += 1; + } } } - DynamicNode::Fragment(nodes) => { - for child in nodes { - self.render_template(buf, dom, child)?; - } - } - - DynamicNode::Placeholder(_) => { - if self.pre_render { - write!(buf, "", self.dynamic_node_id)?; - self.dynamic_node_id += 1; - } - } - }, + } Segment::PreRendered(contents) => write!(buf, "{contents}")?, + Segment::PreRenderedMaybeEscaped { + value, + renderer_if_escaped, + } => { + if *renderer_if_escaped == parent_escaped { + write!(buf, "{value}")?; + } + } Segment::StyleMarker { inside_style_tag } => { if !accumulated_dynamic_styles.is_empty() { @@ -266,6 +283,7 @@ impl Renderer { #[test] fn to_string_works() { + use crate::cache::EscapeText; use dioxus::prelude::*; fn app() -> Element { @@ -311,13 +329,22 @@ fn to_string_works() { PreRendered(">".to_string()), InnerHtmlMarker, PreRendered("Hello world 1 -->".to_string()), - Node(0), + Node { + index: 0, + escape_text: EscapeText::Escape + }, PreRendered( "<-- Hello world 2
nest 1
nest 2
" .to_string() ), - Node(1), - Node(2), + Node { + index: 1, + escape_text: EscapeText::Escape + }, + Node { + index: 2, + escape_text: EscapeText::Escape + }, PreRendered("".to_string()) ] ); @@ -331,6 +358,7 @@ fn to_string_works() { #[test] fn empty_for_loop_works() { + use crate::cache::EscapeText; use dioxus::prelude::*; fn app() -> Element { @@ -360,7 +388,10 @@ fn empty_for_loop_works() { RootNodeMarker, PreRendered("\"".to_string()), PreRendered(">".to_string()), - Node(0), + Node { + index: 0, + escape_text: EscapeText::Escape + }, PreRendered("".to_string()) ] ); @@ -444,7 +475,11 @@ pub(crate) fn write_attribute( ) -> std::fmt::Result { let name = &attr.name; match &attr.value { - AttributeValue::Text(value) => write!(buf, " {name}=\"{value}\""), + AttributeValue::Text(value) => write!( + buf, + " {name}=\"{}\"", + askama_escape::escape(value, askama_escape::Html) + ), AttributeValue::Bool(value) => write!(buf, " {name}={value}"), AttributeValue::Int(value) => write!(buf, " {name}={value}"), AttributeValue::Float(value) => write!(buf, " {name}={value}"), @@ -457,7 +492,9 @@ pub(crate) fn write_value_unquoted( value: &AttributeValue, ) -> std::fmt::Result { match value { - AttributeValue::Text(value) => write!(buf, "{}", value), + AttributeValue::Text(value) => { + write!(buf, "{}", askama_escape::escape(value, askama_escape::Html)) + } AttributeValue::Bool(value) => write!(buf, "{}", value), AttributeValue::Int(value) => write!(buf, "{}", value), AttributeValue::Float(value) => write!(buf, "{}", value), diff --git a/packages/ssr/tests/escape.rs b/packages/ssr/tests/escape.rs new file mode 100644 index 000000000..310a7a907 --- /dev/null +++ b/packages/ssr/tests/escape.rs @@ -0,0 +1,249 @@ +use dioxus::prelude::*; + +#[test] +fn escape_static_values() { + fn app() -> Element { + rsx! { input { disabled: "\">
" } } + } + + let mut dom = VirtualDom::new(app); + dom.rebuild(&mut dioxus_core::NoOpMutations); + + assert_eq!( + dioxus_ssr::pre_render(&dom), + "" + ); +} + +#[test] +fn escape_dynamic_values() { + fn app() -> Element { + let disabled = "\">
"; + rsx! { input { disabled } } + } + + let mut dom = VirtualDom::new(app); + dom.rebuild(&mut dioxus_core::NoOpMutations); + + assert_eq!( + dioxus_ssr::pre_render(&dom), + "" + ); +} + +#[test] +fn escape_static_style() { + fn app() -> Element { + rsx! { div { width: "\">
" } } + } + + let mut dom = VirtualDom::new(app); + dom.rebuild(&mut dioxus_core::NoOpMutations); + + assert_eq!( + dioxus_ssr::pre_render(&dom), + "
" + ); +} + +#[test] +fn escape_dynamic_style() { + fn app() -> Element { + let width = "\">
"; + rsx! { div { width } } + } + + let mut dom = VirtualDom::new(app); + dom.rebuild(&mut dioxus_core::NoOpMutations); + + assert_eq!( + dioxus_ssr::pre_render(&dom), + "
" + ); +} + +#[test] +fn escape_static_text() { + fn app() -> Element { + rsx! { + div { + "\">
" + } + } + } + + let mut dom = VirtualDom::new(app); + dom.rebuild(&mut dioxus_core::NoOpMutations); + + assert_eq!( + dioxus_ssr::pre_render(&dom), + "
"><div>
" + ); +} + +#[test] +fn escape_dynamic_text() { + fn app() -> Element { + let text = "\">
"; + rsx! { + div { + {text} + } + } + } + + let mut dom = VirtualDom::new(app); + dom.rebuild(&mut dioxus_core::NoOpMutations); + + assert_eq!( + dioxus_ssr::pre_render(&dom), + "
"><div>
" + ); +} + +#[test] +fn don_t_escape_static_scripts() { + fn app() -> Element { + rsx! { + script { + "console.log('hello world');" + } + } + } + + let mut dom = VirtualDom::new(app); + dom.rebuild(&mut dioxus_core::NoOpMutations); + + assert_eq!( + dioxus_ssr::pre_render(&dom), + "" + ); +} + +#[test] +fn don_t_escape_dynamic_scripts() { + fn app() -> Element { + let script = "console.log('hello world');"; + rsx! { + script { + {script} + } + } + } + + let mut dom = VirtualDom::new(app); + dom.rebuild(&mut dioxus_core::NoOpMutations); + + assert_eq!( + dioxus_ssr::pre_render(&dom), + "" + ); +} + +#[test] +fn don_t_escape_static_styles() { + fn app() -> Element { + rsx! { + style { + "body {{ background-color: red; }}" + } + } + } + + let mut dom = VirtualDom::new(app); + dom.rebuild(&mut dioxus_core::NoOpMutations); + + assert_eq!( + dioxus_ssr::pre_render(&dom), + "" + ); +} + +#[test] +fn don_t_escape_dynamic_styles() { + fn app() -> Element { + let style = "body { font-family: \"sans-serif\"; }"; + rsx! { + style { + {style} + } + } + } + + let mut dom = VirtualDom::new(app); + dom.rebuild(&mut dioxus_core::NoOpMutations); + + assert_eq!( + dioxus_ssr::pre_render(&dom), + "" + ); +} + +#[test] +fn don_t_escape_static_fragment_styles() { + fn app() -> Element { + let style_element = rsx! { "body {{ font-family: \"sans-serif\"; }}" }; + rsx! { + style { + {style_element} + } + } + } + + let mut dom = VirtualDom::new(app); + dom.rebuild(&mut dioxus_core::NoOpMutations); + + assert_eq!( + dioxus_ssr::pre_render(&dom), + "" + ); +} + +#[test] +fn escape_static_component_fragment_div() { + #[component] + fn StyleContents() -> Element { + rsx! { "body {{ font-family: \"sans-serif\"; }}" } + } + + fn app() -> Element { + rsx! { + div { + StyleContents {} + } + } + } + + let mut dom = VirtualDom::new(app); + dom.rebuild(&mut dioxus_core::NoOpMutations); + + assert_eq!( + dioxus_ssr::pre_render(&dom), + "
body { font-family: "sans-serif"; }
" + ); +} + +#[test] +fn escape_dynamic_component_fragment_div() { + #[component] + fn StyleContents() -> Element { + let dynamic = "body { font-family: \"sans-serif\"; }"; + rsx! { "{dynamic}" } + } + + fn app() -> Element { + rsx! { + div { + StyleContents {} + } + } + } + + let mut dom = VirtualDom::new(app); + dom.rebuild(&mut dioxus_core::NoOpMutations); + + assert_eq!( + dioxus_ssr::pre_render(&dom), + "
body { font-family: "sans-serif"; }
" + ); +}