fix: html script, style, and comment escaping

.changeset/blue-dragons-see.md

@@ -0,0 +1,14 @@
+---
+"marko": patch
+"@marko/runtime-tags": patch
+---
+
+Fix escaping issue for dynamic text interpolation inside `<script>`, `<style>`, `<html-script>` and `<html-style>` tags.
+
+The issue was that the escaping logic for those tags used a CASE SENSITIVE search for the closing tag which could be bypassed like so:
+
+```marko
+<script>${"</SCRIPT><img src=x onerror=alert('uh oh')>"}</script>
+```
+
+Note that `script` and `style` there should _never_ render unsanitized user defined values, regardless of wether or not the closing tag is escaped, since these are conceptually just "eval".

.changeset/frank-memes-knock.md

@@ -0,0 +1,13 @@
+---
+"marko": patch
+"@marko/runtime-tags": patch
+---
+
+Fix escaping for `<html-comment>` tag.
+Previously this tag relied on normal xml escaping which looks for `<`.
+This PR updates to have a special escape for `<html-comment>` tags that replaces `>` instead.
+
+```marko
+// Previously incorrectly escaped.
+<html-comment>${">Uh oh"}</html-comment>
+```

packages/runtime-class/src/runtime/html/helpers/escape-comment-placeholder.js

@@ -0,0 +1,17 @@
+"use strict";
+const unsafeCharsReg = />/g;
+const replaceMatch = () => ">";
+const escape = (str) =>
+  unsafeCharsReg.test(str) ? str.replace(unsafeCharsReg, replaceMatch) : str;
+
+/**
+ * Escapes content placed inside an <html-comment> tag.
+ *
+ * For example:
+ * <html-comment>${userInput}</html-comment>
+ *
+ * Without escaping, a value of `><script>alert(1)</script><!--` would close the comment early.
+ */
+module.exports = function escapeCommentHelper(value) {
+  return escape(value + "");
+};

packages/runtime-class/src/runtime/html/helpers/escape-script-placeholder.js

@@ -1,5 +1,5 @@
 "use strict";
-const unsafeCharsReg = /<\/script/g;
+const unsafeCharsReg = /<\/script/gi;
 const replaceMatch = () => "\\x3C/script";
 const escape = (str) =>
   unsafeCharsReg.test(str) ? str.replace(unsafeCharsReg, replaceMatch) : str;

packages/runtime-class/src/runtime/html/helpers/escape-style-placeholder.js

@@ -1,5 +1,5 @@
 "use strict";
-const unsafeCharsReg = /<\/style/g;
+const unsafeCharsReg = /<\/style/gi;
 const replaceMatch = () => "\\3C/style";
 const escape = (str) =>
   unsafeCharsReg.test(str) ? str.replace(unsafeCharsReg, replaceMatch) : str;

packages/runtime-class/src/translator/taglib/core/translate-html-comment.js

@@ -3,6 +3,7 @@ import {
   assertNoArgs,
   assertNoAttributes,
   assertNoParams,
+  importDefault,
 } from "@marko/compiler/babel-utils";
 
 import writeHTML from "../../util/html-out-write";
@@ -15,11 +16,30 @@ export function enter(path) {
   assertNoAttributes(path);
 
   if (path.hub.file.markoOpts.output === "html") {
-    path.replaceWithMultiple([
-      writeHTML`<!--`,
-      ...path.node.body.body,
-      writeHTML`-->`,
-    ]);
+    const { file } = path.hub;
+    const nodes = [writeHTML`<!--`];
+
+    for (const child of path.node.body.body) {
+      if (t.isMarkoText(child)) {
+        nodes.push(child);
+      } else if (t.isMarkoPlaceholder(child)) {
+        const escapeFnModule = child.escape
+          ? "marko/src/runtime/html/helpers/escape-comment-placeholder.js"
+          : "marko/src/runtime/helpers/to-string.js";
+        const escapeFnAlias = child.escape
+          ? "marko_escapeComment"
+          : "marko_to_string";
+        nodes.push(
+          writeHTML`${t.callExpression(
+            importDefault(file, escapeFnModule, escapeFnAlias),
+            [child.value],
+          )}`,
+        );
+      }
+    }
+
+    nodes.push(writeHTML`-->`);
+    path.replaceWithMultiple(nodes.filter(Boolean));
   } else {
     const templateQuasis = [];
     const templateExpressions = [];

packages/runtime-class/test/render/fixtures/escape-comment/expected.html

@@ -0,0 +1 @@
+<!----&gt;-->

packages/runtime-class/test/render/fixtures/escape-comment/template.marko

@@ -0,0 +1 @@
+<html-comment>${input.text}</html-comment>

packages/runtime-class/test/render/fixtures/escape-comment/test.js

@@ -0,0 +1,3 @@
+exports.templateData = {
+  text: "-->",
+};

packages/runtime-class/test/render/fixtures/escape-comment/vdom-expected.html

@@ -0,0 +1 @@
+<!--"-->"-->

packages/runtime-class/test/render/fixtures/escape-script-case/expected.html

@@ -0,0 +1,3 @@
+<script>
+    var foo = {"name":"Evil \x3C/script>"};
+</script><pre>{"name":"Evil &lt;/SCRIPT>"}</pre>

packages/runtime-class/test/render/fixtures/escape-script-case/template.marko

@@ -0,0 +1,4 @@
+<script>
+    var foo = ${JSON.stringify(input.foo)};
+</script>
+<pre>${JSON.stringify(input.foo)}</pre>

packages/runtime-class/test/render/fixtures/escape-script-case/test.js

@@ -0,0 +1,5 @@
+exports.templateData = {
+  foo: {
+    name: "Evil </SCRIPT>",
+  },
+};

packages/runtime-class/test/render/fixtures/escape-script-case/vdom-expected.html

@@ -0,0 +1,4 @@
+<SCRIPT>
+  "\n    var foo = {\"name\":\"Evil </SCRIPT>\"};\n"
+<PRE>
+  "{\"name\":\"Evil </SCRIPT>\"}"

packages/runtime-class/test/render/fixtures/escape-style-case/expected.html

@@ -0,0 +1,5 @@
+<style>
+    #foo {
+        background-color:\3C/style><script>alert("evil");</script>;
+    }
+</style>

packages/runtime-class/test/render/fixtures/escape-style-case/template.marko

@@ -0,0 +1,5 @@
+<style>
+    #foo {
+        background-color:${input.color};
+    }
+</style>

packages/runtime-class/test/render/fixtures/escape-style-case/test.js

@@ -0,0 +1,3 @@
+exports.templateData = {
+  color: '</STYLE><script>alert("evil");</script>',
+};

packages/runtime-class/test/render/fixtures/escape-style-case/vdom-expected.html

@@ -0,0 +1,2 @@
+<STYLE>
+  "\n    #foo {\n        background-color:</STYLE><script>alert(\"evil\");</script>;\n    }\n"

packages/runtime-tags/src/__tests__/fixtures/html-comment-counter/__snapshots__/html.expected/template.js

@@ -3,7 +3,7 @@ export default _._template("__tests__/template.marko", input => {
   _._scope_reason();
   const $scope0_id = _._scope_id();
   let count = 0;
-  _._html(`<div><button>${_._escape(count)}${_._el_resume($scope0_id, "#text/1")}</button>${_._el_resume($scope0_id, "#button/0")}<!--${_._escape(count)} + ${_._escape(count)} = ${_._escape(count + count)}-->${_._el_resume($scope0_id, "#comment/2")}</div>`);
+  _._html(`<div><button>${_._escape(count)}${_._el_resume($scope0_id, "#text/1")}</button>${_._el_resume($scope0_id, "#button/0")}<!--${_._escape_comment(count)} + ${_._escape_comment(count)} = ${_._escape_comment(count + count)}-->${_._el_resume($scope0_id, "#comment/2")}</div>`);
   _._script($scope0_id, "__tests__/template.marko_0_count");
   _._scope($scope0_id, {
     count

packages/runtime-tags/src/__tests__/fixtures/html-comment-placeholder/__snapshots__/.name-cache.json

@@ -0,0 +1,5 @@
+{
+  "vars": {
+    "props": {}
+  }
+}

packages/runtime-tags/src/__tests__/fixtures/html-comment-placeholder/__snapshots__/csr-sanitized.expected.md

@@ -0,0 +1 @@
+# Render

packages/runtime-tags/src/__tests__/fixtures/html-comment-placeholder/__snapshots__/csr.expected.md

@@ -0,0 +1,9 @@
+# Render
+```html
+<!----&gt;-->
+```
+
+# Mutations
+```
+INSERT #comment
+```

packages/runtime-tags/src/__tests__/fixtures/html-comment-placeholder/__snapshots__/dom.expected/template.hydrate.js

@@ -0,0 +1 @@
+// size: 0

packages/runtime-tags/src/__tests__/fixtures/html-comment-placeholder/__snapshots__/dom.expected/template.js

@@ -0,0 +1,7 @@
+export const $template = "<!---->";
+export const $walks = /* get, over(1) */" b";
+import * as _ from "@marko/runtime-tags/debug/dom";
+export function $setup($scope) {
+  _._text($scope["#comment/0"], `${_._to_text("-->")}`);
+}
+export default /* @__PURE__ */_._template("__tests__/template.marko", $template, $walks, $setup);

packages/runtime-tags/src/__tests__/fixtures/html-comment-placeholder/__snapshots__/html.expected/template.js

@@ -0,0 +1,6 @@
+import * as _ from "@marko/runtime-tags/debug/html";
+export default _._template("__tests__/template.marko", input => {
+  _._scope_reason();
+  const $scope0_id = _._scope_id();
+  _._html(`<!--${_._escape_comment("-->")}-->`);
+}, 1);

packages/runtime-tags/src/__tests__/fixtures/html-comment-placeholder/__snapshots__/resume-sanitized.expected.md

@@ -0,0 +1 @@
+# Render

packages/runtime-tags/src/__tests__/fixtures/html-comment-placeholder/__snapshots__/resume.expected.md

@@ -0,0 +1,8 @@
+# Render
+```html
+<!----&gt;-->
+<html>
+  <head />
+  <body />
+</html>
+```

packages/runtime-tags/src/__tests__/fixtures/html-comment-placeholder/__snapshots__/ssr-sanitized.expected.md

@@ -0,0 +1 @@
+# Render End

packages/runtime-tags/src/__tests__/fixtures/html-comment-placeholder/__snapshots__/ssr.expected.md

@@ -0,0 +1,21 @@
+# Write
+```html
+  <!----&gt;-->
+```
+
+# Render End
+```html
+<!----&gt;-->
+<html>
+  <head />
+  <body />
+</html>
+```
+
+# Mutations
+```
+INSERT #comment
+INSERT html
+INSERT html/head
+INSERT html/body
+```

packages/runtime-tags/src/__tests__/fixtures/html-comment-placeholder/template.marko

@@ -0,0 +1 @@
+<html-comment>${"-->"}</html-comment>

packages/runtime-tags/src/__tests__/fixtures/html-comment-placeholder/test.ts

@@ -0,0 +1,5 @@
+import type { TestConfig } from "../../main.test";
+
+export const config: TestConfig = {
+  steps: [{}],
+};

packages/runtime-tags/src/__tests__/fixtures/html-script-injection/__snapshots__/.name-cache.json

@@ -0,0 +1,5 @@
+{
+  "vars": {
+    "props": {}
+  }
+}

packages/runtime-tags/src/__tests__/fixtures/html-script-injection/__snapshots__/csr-sanitized.expected.md

@@ -0,0 +1 @@
+# Render

packages/runtime-tags/src/__tests__/fixtures/html-script-injection/__snapshots__/csr.expected.md

@@ -0,0 +1,11 @@
+# Render
+```html
+<script>
+  var x = '&lt;/SCRIPT&gt;'
+</script>
+```
+
+# Mutations
+```
+INSERT script
+```

packages/runtime-tags/src/__tests__/fixtures/html-script-injection/__snapshots__/dom.expected/template.hydrate.js

@@ -0,0 +1 @@
+// size: 0

packages/runtime-tags/src/__tests__/fixtures/html-script-injection/__snapshots__/dom.expected/template.js

@@ -0,0 +1,9 @@
+export const $template = "<script></script>";
+export const $walks = /* get, over(1) */" b";
+import * as _ from "@marko/runtime-tags/debug/dom";
+const $injection = /* @__PURE__ */_._let("injection/1", $scope => _._text_content($scope["#script/0"], `var x = '${_._to_text($scope.injection)}'`));
+export function $setup($scope) {
+  _._attr_nonce($scope, "#script/0");
+  $injection($scope, "</SCRIPT>");
+}
+export default /* @__PURE__ */_._template("__tests__/template.marko", $template, $walks, $setup);

packages/runtime-tags/src/__tests__/fixtures/html-script-injection/__snapshots__/html.expected/template.js

@@ -0,0 +1,8 @@
+import * as _ from "@marko/runtime-tags/debug/html";
+export default _._template("__tests__/template.marko", input => {
+  _._scope_reason();
+  const $scope0_id = _._scope_id();
+  let injection = "</SCRIPT>";
+  _._html(`<script${_._attr_nonce()}>var x = '${_._escape_script(injection)}'</script>`);
+  _._resume_branch($scope0_id);
+}, 1);

packages/runtime-tags/src/__tests__/fixtures/html-script-injection/__snapshots__/resume-sanitized.expected.md

@@ -0,0 +1 @@
+# Render

packages/runtime-tags/src/__tests__/fixtures/html-script-injection/__snapshots__/resume.expected.md

@@ -0,0 +1,11 @@
+# Render
+```html
+<html>
+  <head>
+    <script>
+      var x = '\x3C/script&gt;'
+    </script>
+  </head>
+  <body />
+</html>
+```

packages/runtime-tags/src/__tests__/fixtures/html-script-injection/__snapshots__/ssr-sanitized.expected.md

@@ -0,0 +1 @@
+# Render End

packages/runtime-tags/src/__tests__/fixtures/html-script-injection/__snapshots__/ssr.expected.md

@@ -0,0 +1,25 @@
+# Write
+```html
+  <script>var x = '\x3C/script>'</script>
+```
+
+# Render End
+```html
+<html>
+  <head>
+    <script>
+      var x = '\x3C/script&gt;'
+    </script>
+  </head>
+  <body />
+</html>
+```
+
+# Mutations
+```
+INSERT html
+INSERT html/head
+INSERT html/head/script
+INSERT html/head/script/#text
+INSERT html/body
+```

packages/runtime-tags/src/__tests__/fixtures/html-script-injection/template.marko

@@ -0,0 +1,2 @@
+<let/injection = "</SCRIPT>" />
+<html-script>var x = '${injection}'</html-script>

packages/runtime-tags/src/__tests__/fixtures/html-script-injection/test.ts

@@ -0,0 +1,5 @@
+import type { TestConfig } from "../../main.test";
+
+export const config: TestConfig = {
+  steps: [{}],
+};

packages/runtime-tags/src/__tests__/fixtures/html-style-injection/__snapshots__/.name-cache.json

@@ -0,0 +1,5 @@
+{
+  "vars": {
+    "props": {}
+  }
+}

packages/runtime-tags/src/__tests__/fixtures/html-style-injection/__snapshots__/csr-sanitized.expected.md

@@ -0,0 +1 @@
+# Render

packages/runtime-tags/src/__tests__/fixtures/html-style-injection/__snapshots__/csr.expected.md

@@ -0,0 +1,11 @@
+# Render
+```html
+<style>
+  .evil { content: '&lt;/STYLE&gt;'; }
+</style>
+```
+
+# Mutations
+```
+INSERT style
+```

packages/runtime-tags/src/__tests__/fixtures/html-style-injection/__snapshots__/dom.expected/template.hydrate.js

@@ -0,0 +1 @@
+// size: 0

packages/runtime-tags/src/__tests__/fixtures/html-style-injection/__snapshots__/dom.expected/template.js

@@ -0,0 +1,9 @@
+export const $template = "<style></style>";
+export const $walks = /* get, over(1) */" b";
+import * as _ from "@marko/runtime-tags/debug/dom";
+const $injection = /* @__PURE__ */_._let("injection/1", $scope => _._text_content($scope["#style/0"], `.evil { content: '${_._to_text($scope.injection)}'; }`));
+export function $setup($scope) {
+  _._attr_nonce($scope, "#style/0");
+  $injection($scope, "</STYLE>");
+}
+export default /* @__PURE__ */_._template("__tests__/template.marko", $template, $walks, $setup);

packages/runtime-tags/src/__tests__/fixtures/html-style-injection/__snapshots__/html.expected/template.js

@@ -0,0 +1,8 @@
+import * as _ from "@marko/runtime-tags/debug/html";
+export default _._template("__tests__/template.marko", input => {
+  _._scope_reason();
+  const $scope0_id = _._scope_id();
+  let injection = "</STYLE>";
+  _._html(`<style${_._attr_nonce()}>.evil { content: '${_._escape_style(injection)}'; }</style>`);
+  _._resume_branch($scope0_id);
+}, 1);

packages/runtime-tags/src/__tests__/fixtures/html-style-injection/__snapshots__/resume-sanitized.expected.md

@@ -0,0 +1 @@
+# Render