Browse Source

Fix escaped text in ssr stylesheets and scripts (#3933)

Evan Almloff 2 months ago
parent
commit
4f74ef81d1

+ 6 - 0
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);
     });
 

+ 1 - 1
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" });

+ 107 - 13
packages/ssr/src/cache.rs

@@ -78,14 +78,52 @@ impl AddAssign<Segment> 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<usize>,
     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, "</{tag}>")?;
             }
@@ -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(())

+ 78 - 41
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();
-
-                            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)?
-                        }
-                    }
-                    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, "<!--node-id{}-->", self.dynamic_node_id)?;
-                            self.dynamic_node_id += 1;
+                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(*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, "<!--node-id{}-->", self.dynamic_node_id)?;
+                                self.dynamic_node_id += 1;
+                            }
 
-                        write!(
-                            buf,
-                            "{}",
-                            askama_escape::escape(&text.value, askama_escape::Html)
-                        )?;
+                            if escaped {
+                                write!(
+                                    buf,
+                                    "{}",
+                                    askama_escape::escape(&text.value, askama_escape::Html)
+                                )?;
+                            } else {
+                                write!(buf, "{}", text.value)?;
+                            }
 
-                        if self.pre_render {
-                            write!(buf, "<!--#-->")?;
+                            if self.pre_render {
+                                write!(buf, "<!--#-->")?;
+                            }
                         }
-                    }
-                    DynamicNode::Fragment(nodes) => {
-                        for child in nodes {
-                            self.render_template(buf, dom, child)?;
+                        DynamicNode::Fragment(nodes) => {
+                            for child in nodes {
+                                self.render_template(buf, dom, child, escaped)?;
+                            }
                         }
-                    }
 
-                    DynamicNode::Placeholder(_) => {
-                        if self.pre_render {
-                            write!(buf, "<!--placeholder{}-->", self.dynamic_node_id)?;
-                            self.dynamic_node_id += 1;
+                        DynamicNode::Placeholder(_) => {
+                            if self.pre_render {
+                                write!(buf, "<!--placeholder{}-->", 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 --&gt;".to_string()),
-                    Node(0),
+                    Node {
+                        index: 0,
+                        escape_text: EscapeText::Escape
+                    },
                     PreRendered(
                         "&lt;-- Hello world 2<div>nest 1</div><div></div><div>nest 2</div>"
                             .to_string()
                     ),
-                    Node(1),
-                    Node(2),
+                    Node {
+                        index: 1,
+                        escape_text: EscapeText::Escape
+                    },
+                    Node {
+                        index: 2,
+                        escape_text: EscapeText::Escape
+                    },
                     PreRendered("</div>".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("</div>".to_string())
                 ]
             );
@@ -444,7 +475,11 @@ pub(crate) fn write_attribute<W: Write + ?Sized>(
 ) -> 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<W: Write + ?Sized>(
     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),

+ 249 - 0
packages/ssr/tests/escape.rs

@@ -0,0 +1,249 @@
+use dioxus::prelude::*;
+
+#[test]
+fn escape_static_values() {
+    fn app() -> Element {
+        rsx! { input { disabled: "\"><div>" } }
+    }
+
+    let mut dom = VirtualDom::new(app);
+    dom.rebuild(&mut dioxus_core::NoOpMutations);
+
+    assert_eq!(
+        dioxus_ssr::pre_render(&dom),
+        "<input disabled=\"&quot;&gt;&lt;div&gt;\" data-node-hydration=\"0\"/>"
+    );
+}
+
+#[test]
+fn escape_dynamic_values() {
+    fn app() -> Element {
+        let disabled = "\"><div>";
+        rsx! { input { disabled } }
+    }
+
+    let mut dom = VirtualDom::new(app);
+    dom.rebuild(&mut dioxus_core::NoOpMutations);
+
+    assert_eq!(
+        dioxus_ssr::pre_render(&dom),
+        "<input disabled=\"&quot;&gt;&lt;div&gt;\" data-node-hydration=\"0\"/>"
+    );
+}
+
+#[test]
+fn escape_static_style() {
+    fn app() -> Element {
+        rsx! { div { width: "\"><div>" } }
+    }
+
+    let mut dom = VirtualDom::new(app);
+    dom.rebuild(&mut dioxus_core::NoOpMutations);
+
+    assert_eq!(
+        dioxus_ssr::pre_render(&dom),
+        "<div style=\"width:&quot;&gt;&lt;div&gt;;\" data-node-hydration=\"0\"></div>"
+    );
+}
+
+#[test]
+fn escape_dynamic_style() {
+    fn app() -> Element {
+        let width = "\"><div>";
+        rsx! { div { width } }
+    }
+
+    let mut dom = VirtualDom::new(app);
+    dom.rebuild(&mut dioxus_core::NoOpMutations);
+
+    assert_eq!(
+        dioxus_ssr::pre_render(&dom),
+        "<div style=\"width:&quot;&gt;&lt;div&gt;;\" data-node-hydration=\"0\"></div>"
+    );
+}
+
+#[test]
+fn escape_static_text() {
+    fn app() -> Element {
+        rsx! {
+            div {
+                "\"><div>"
+            }
+        }
+    }
+
+    let mut dom = VirtualDom::new(app);
+    dom.rebuild(&mut dioxus_core::NoOpMutations);
+
+    assert_eq!(
+        dioxus_ssr::pre_render(&dom),
+        "<div data-node-hydration=\"0\">&quot;&gt;&lt;div&gt;</div>"
+    );
+}
+
+#[test]
+fn escape_dynamic_text() {
+    fn app() -> Element {
+        let text = "\"><div>";
+        rsx! {
+            div {
+                {text}
+            }
+        }
+    }
+
+    let mut dom = VirtualDom::new(app);
+    dom.rebuild(&mut dioxus_core::NoOpMutations);
+
+    assert_eq!(
+        dioxus_ssr::pre_render(&dom),
+        "<div data-node-hydration=\"0\"><!--node-id1-->&quot;&gt;&lt;div&gt;<!--#--></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),
+        "<script data-node-hydration=\"0\">console.log('hello world');</script>"
+    );
+}
+
+#[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),
+        "<script data-node-hydration=\"0\"><!--node-id1-->console.log('hello world');<!--#--></script>"
+    );
+}
+
+#[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),
+        "<style data-node-hydration=\"0\">body { background-color: red; }</style>"
+    );
+}
+
+#[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),
+        "<style data-node-hydration=\"0\"><!--node-id1-->body { font-family: \"sans-serif\"; }<!--#--></style>"
+    );
+}
+
+#[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),
+        "<style data-node-hydration=\"0\"><!--node-id1-->body { font-family: \"sans-serif\"; }<!--#--></style>"
+    );
+}
+
+#[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),
+        "<div data-node-hydration=\"0\"><!--node-id1-->body { font-family: &quot;sans-serif&quot;; }<!--#--></div>"
+    );
+}
+
+#[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),
+        "<div data-node-hydration=\"0\"><!--node-id1-->body { font-family: &quot;sans-serif&quot;; }<!--#--></div>"
+    );
+}