Skip to content

Commit 05a17cb

Browse files
vanceingallsvanceingalls
committed
chore: merge in the main restack from #3210 
2 parents 53f1320 + e259099 commit 05a17cb

5 files changed

Lines changed: 295 additions & 17 deletions

File tree

‎packages/core/src/compiler/compositionScoping.test.ts‎

Lines changed: 137 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, it, vi } from "vitest";
22
import { parseHTML } from "linkedom";
33
import {
4+
buildVariablesByCompScript,
45
scopeCssToComposition,
56
wrapInlineScriptWithErrorBoundary,
67
wrapScopedCompositionScript,
@@ -699,13 +700,18 @@ window.__afterTimeline = window.__timelines.scene;
699700
});
700701

701702
it("wraps unscoped composition script source as a string literal", () => {
703+
const source = 'window.payload = "</script><script>window.pwned = true;</script>";';
702704
const wrapped = wrapInlineScriptWithErrorBoundary(
703-
'window.payload = "</script><script>window.pwned = true;</script>";',
705+
source,
704706
"[HyperFrames] composition script error:",
705707
);
706708

707709
expect(wrapped).toContain("Function(");
708-
expect(wrapped).toContain('\\"</script><script>window.pwned = true;</script>\\"');
710+
// The literal carries the source verbatim, with `<` escaped so it cannot end the
711+
// raw-text `<script>` this is emitted into.
712+
expect(wrapped).not.toContain("</script");
713+
const literal = /Function\((".*")\)/.exec(wrapped)?.[1];
714+
expect(JSON.parse(literal ?? "")).toBe(source);
709715
});
710716

711717
it("rewrites #id CSS selectors to [data-hf-authored-id] when authoredRootId is provided", () => {
@@ -886,3 +892,132 @@ window.__timelines['intro'] = tl;
886892
expect(gsapTargets).toEqual([["HELLO"]]);
887893
});
888894
});
895+
896+
/**
897+
* The emitted statement is placed inside a `<script>` element, and `<script>` is a
898+
* RAW TEXT element: HTML serialization does not escape its content and the tokenizer
899+
* closes it at the first `</script`. `JSON.stringify` escapes `"` and `\` but not `/`,
900+
* so an unescaped variable value could close the element and have the remainder parsed
901+
* as markup — turning composition data into executable script.
902+
*/
903+
/**
904+
* Every payload leads with a benign `<` before its `</script`, so escaping only the
905+
* first `<` is not enough to pass: that pins the `/g` flag on the escape rather than
906+
* merely "an escape ran". A lone `<` in a value is the common case (`a < b`, `<em>`),
907+
* so a payload whose breakout is not the first `<` is the realistic one.
908+
*/
909+
const SCRIPT_BREAKOUT = "x<y</script><script>window.__pwned=1//";
910+
911+
/** Serialize into a document the way the compilers do, then re-parse it. */
912+
function scriptsAfterRoundTrip(body: string): string[] {
913+
const { document } = parseHTML("<!doctype html><html><head></head><body></body></html>");
914+
const el = document.createElement("script");
915+
el.textContent = body;
916+
document.body.appendChild(el);
917+
const { document: reparsed } = parseHTML(document.toString());
918+
return [...reparsed.querySelectorAll("script")].map((s) => s.textContent ?? "");
919+
}
920+
921+
describe("buildVariablesByCompScript — <script> breakout", () => {
922+
it("does not let a variable VALUE close the script element", () => {
923+
const body = buildVariablesByCompScript({
924+
"comp-a": { greeting: SCRIPT_BREAKOUT },
925+
});
926+
expect(body).not.toBeNull();
927+
expect(body).not.toContain("</script");
928+
expect(scriptsAfterRoundTrip(body ?? "")).toHaveLength(1);
929+
});
930+
931+
it("does not let a variable KEY close the script element", () => {
932+
const body = buildVariablesByCompScript({
933+
"comp-a": { [SCRIPT_BREAKOUT]: "x" },
934+
});
935+
expect(body).not.toContain("</script");
936+
expect(scriptsAfterRoundTrip(body ?? "")).toHaveLength(1);
937+
});
938+
939+
it("does not let a COMP ID close the script element", () => {
940+
const body = buildVariablesByCompScript({
941+
[SCRIPT_BREAKOUT]: { a: "x" },
942+
});
943+
expect(body).not.toContain("</script");
944+
expect(scriptsAfterRoundTrip(body ?? "")).toHaveLength(1);
945+
});
946+
947+
it("keeps the value byte-identical once executed — the escape is transparent", () => {
948+
// Run the statement the way the browser does rather than string-slicing it.
949+
const variables = { "comp-a": { greeting: "a </script> b <em>c</em>" } };
950+
const body = buildVariablesByCompScript(variables) ?? "";
951+
const fakeWindow: Record<string, unknown> = {};
952+
new Function("window", body)(fakeWindow);
953+
expect(fakeWindow.__hfVariablesByComp).toEqual(variables);
954+
});
955+
956+
it("returns null when there are no per-instance values", () => {
957+
expect(buildVariablesByCompScript({})).toBeNull();
958+
});
959+
});
960+
961+
/**
962+
* The variables table is not the only attacker-reachable literal emitted into a
963+
* `<script>`: the wrapper the sub-composition scripts run inside embeds the
964+
* composition id four times over (directly, as the timeline id, and inside two
965+
* derived selector patterns), plus the authored root id, the scope-selector
966+
* override and the error label. All of them are emitted into the same raw-text
967+
* element, so each has to survive a serialize/reparse round trip.
968+
*/
969+
describe("wrapScopedCompositionScript — <script> breakout via the wrapper literals", () => {
970+
const LABEL = "[HyperFrames] composition script error:";
971+
972+
it("does not let a COMP ID close the script element", () => {
973+
const body = wrapScopedCompositionScript("console.log(1);", SCRIPT_BREAKOUT);
974+
expect(body).not.toContain("</script");
975+
expect(scriptsAfterRoundTrip(body)).toHaveLength(1);
976+
});
977+
978+
it("keeps the comp id byte-identical — the escape is transparent", () => {
979+
const body = wrapScopedCompositionScript("console.log(1);", SCRIPT_BREAKOUT);
980+
const literal = /var __hfCompId = (.*);/.exec(body)?.[1];
981+
expect(literal).toBeDefined();
982+
expect(JSON.parse(literal ?? "")).toBe(SCRIPT_BREAKOUT);
983+
});
984+
985+
it("does not let the AUTHORED ROOT ID close the script element", () => {
986+
const body = wrapScopedCompositionScript(
987+
"console.log(1);",
988+
"comp-a",
989+
LABEL,
990+
undefined,
991+
"comp-a",
992+
SCRIPT_BREAKOUT,
993+
);
994+
expect(body).not.toContain("</script");
995+
expect(scriptsAfterRoundTrip(body)).toHaveLength(1);
996+
});
997+
998+
it("does not let the SCOPE SELECTOR override close the script element", () => {
999+
const body = wrapScopedCompositionScript("console.log(1);", "comp-a", LABEL, SCRIPT_BREAKOUT);
1000+
expect(body).not.toContain("</script");
1001+
expect(scriptsAfterRoundTrip(body)).toHaveLength(1);
1002+
});
1003+
1004+
it("does not let the ERROR LABEL close the script element", () => {
1005+
const body = wrapScopedCompositionScript("console.log(1);", "comp-a", SCRIPT_BREAKOUT);
1006+
expect(body).not.toContain("</script");
1007+
expect(scriptsAfterRoundTrip(body)).toHaveLength(1);
1008+
});
1009+
});
1010+
1011+
describe("wrapInlineScriptWithErrorBoundary — <script> breakout", () => {
1012+
it("does not let the wrapped SOURCE close the script element", () => {
1013+
const body = wrapInlineScriptWithErrorBoundary(`var a = "${SCRIPT_BREAKOUT}";`, "[err]");
1014+
expect(body).not.toContain("</script");
1015+
expect(scriptsAfterRoundTrip(body)).toHaveLength(1);
1016+
});
1017+
1018+
it("does not let the ERROR LABEL close the script element", () => {
1019+
const body = wrapInlineScriptWithErrorBoundary("var a = 1;", SCRIPT_BREAKOUT);
1020+
expect(body).not.toContain("</script");
1021+
expect(scriptsAfterRoundTrip(body)).toHaveLength(1);
1022+
});
1023+
});

‎packages/core/src/compiler/compositionScoping.ts‎

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,26 @@ export function scopeCssToComposition(
250250
return root.toResult({ map: false }).css;
251251
}
252252

253+
/**
254+
* Serialize a value as a JS literal safe to emit inside a `<script>` element.
255+
*
256+
* `<script>` is a RAW TEXT element: HTML serialization does not escape its
257+
* content, and the tokenizer ends the element at the first `</script` — in any
258+
* string, comment or regex context. `JSON.stringify` escapes `"` and `\` but
259+
* neither `<` nor `/`, so any dynamic literal carrying `</script>` would close
260+
* the element early and have the remainder parsed as markup. Rewriting every
261+
* `<` to `<` removes the only byte that can start a closing tag, and is
262+
* transparent to both `JSON.parse` and the JS string grammar, so the value the
263+
* runtime reads is unchanged.
264+
*
265+
* Every dynamic literal in an emitted script body must go through here: a
266+
* per-value guard on this surface has already been missed once, since the
267+
* composition id reaches the emitted script through four separate literals.
268+
*/
269+
function jsonScriptLiteral(value: unknown): string {
270+
return JSON.stringify(value).replace(/</g, "\\u003c");
271+
}
272+
253273
export function wrapScopedCompositionScript(
254274
source: string,
255275
compositionId: string,
@@ -258,27 +278,27 @@ export function wrapScopedCompositionScript(
258278
timelineCompositionId = compositionId,
259279
authoredRootId?: string | null,
260280
): string {
261-
const compositionIdLiteral = JSON.stringify(compositionId);
262-
const timelineCompositionIdLiteral = JSON.stringify(timelineCompositionId);
263-
const errorLabelLiteral = JSON.stringify(errorLabel);
281+
const compositionIdLiteral = jsonScriptLiteral(compositionId);
282+
const timelineCompositionIdLiteral = jsonScriptLiteral(timelineCompositionId);
283+
const errorLabelLiteral = jsonScriptLiteral(errorLabel);
264284
const escapedCompositionId = escapeRegExp(compositionId);
265-
const authoredRootIdLiteral = JSON.stringify(authoredRootId?.trim() || null);
266-
const scopeSelectorLiteral = JSON.stringify(scopeSelectorOverride ?? null);
267-
const rootSelectorPatternLiteral = JSON.stringify(
285+
const authoredRootIdLiteral = jsonScriptLiteral(authoredRootId?.trim() || null);
286+
const scopeSelectorLiteral = jsonScriptLiteral(scopeSelectorOverride ?? null);
287+
const rootSelectorPatternLiteral = jsonScriptLiteral(
268288
String.raw`\[\s*data-composition-id\s*=\s*(?:"${escapedCompositionId}"|'${escapedCompositionId}')\s*\]`,
269289
);
270-
const timingSelectorPatternLiteral = JSON.stringify(
290+
const timingSelectorPatternLiteral = jsonScriptLiteral(
271291
String.raw`\s*\[\s*data-(?:start|duration)\s*=\s*(?:"[^"]*"|'[^']*')\s*\]`,
272292
);
273-
const authoredRootIdFormsLiteral = JSON.stringify(
293+
const authoredRootIdFormsLiteral = jsonScriptLiteral(
274294
getAuthoredRootIdSelectorForms(authoredRootId?.trim() || ""),
275295
);
276296
return `(function(){
277297
var __hfCompId = ${compositionIdLiteral};
278298
var __hfTimelineCompId = ${timelineCompositionIdLiteral};
279299
var __hfErrorLabel = ${errorLabelLiteral};
280300
var __hfAuthoredRootId = ${authoredRootIdLiteral};
281-
var __hfAuthoredRootAttr = ${JSON.stringify(AUTHORED_ROOT_ID_ATTR)};
301+
var __hfAuthoredRootAttr = ${jsonScriptLiteral(AUTHORED_ROOT_ID_ATTR)};
282302
var __hfEscapeAttr = function(value) {
283303
return (value + "").replace(/\\\\/g, "\\\\\\\\").replace(/"/g, "\\\\\\"");
284304
};
@@ -585,7 +605,7 @@ ${source.replace(/<\/(script)/gi, "<\\/$1")}
585605
}
586606

587607
export function wrapInlineScriptWithErrorBoundary(source: string, errorLabel: string): string {
588-
return `(function(){ try { Function(${JSON.stringify(source)}).call(window); } catch (_err) { console.error(${JSON.stringify(errorLabel)}, _err); } })();`;
608+
return `(function(){ try { Function(${jsonScriptLiteral(source)}).call(window); } catch (_err) { console.error(${jsonScriptLiteral(errorLabel)}, _err); } })();`;
589609
}
590610

591611
/**
@@ -601,10 +621,14 @@ export function wrapInlineScriptWithErrorBoundary(source: string, errorLabel: st
601621
* `getVariables()` returned `{}` only during render — parametrized sub-comps
602622
* silently shipped blank/default text in the final MP4 while snapshot QA passed
603623
* (issue #2064). Both callers now share this one builder so they can't drift.
624+
*
625+
* Values, keys and composition ids are all attacker-reachable, so the whole
626+
* table goes through `jsonScriptLiteral` — see there for why.
604627
*/
605628
export function buildVariablesByCompScript(
606629
variablesByComp: Record<string, Record<string, unknown>>,
607630
): string | null {
608631
if (!variablesByComp || Object.keys(variablesByComp).length === 0) return null;
609-
return `window.__hfVariablesByComp = Object.assign({}, window.__hfVariablesByComp || {}, ${JSON.stringify(variablesByComp)});`;
632+
const json = jsonScriptLiteral(variablesByComp);
633+
return `window.__hfVariablesByComp = Object.assign({}, window.__hfVariablesByComp || {}, ${json});`;
610634
}

‎packages/core/src/compiler/htmlBundler.test.ts‎

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@ import { tmpdir } from "node:os";
44
import { join } from "node:path";
55
import { parseHTML } from "linkedom";
66
import { afterEach, beforeEach, describe, it, expect, vi } from "vitest";
7-
import { bundleToSingleHtml } from "./htmlBundler";
7+
import { bundleToSingleHtml, emitRootCompositionVariableStyles } from "./htmlBundler";
88
import { resetUnknownEnumWarnings } from "../runtime/getVariables";
9+
import { sanitizeCssValue } from "../runtime/applyVariableBindings";
910
import { getHyperframeRuntimeScript } from "../generated/runtime-inline";
1011

1112
function makeTempProject(files: Record<string, string>): string {
@@ -1510,3 +1511,75 @@ describe("bundleToSingleHtml unknown enum values", () => {
15101511
]);
15111512
});
15121513
});
1514+
1515+
/**
1516+
* Composition variable values are emitted as CSS declarations inside a `<style>`
1517+
* element. `<style>` is a RAW TEXT element: HTML serialization does not escape its
1518+
* content and the tokenizer closes it at the first `</style`. An unescaped value could
1519+
* therefore close the element and have the remainder parsed as markup.
1520+
*/
1521+
describe("emitRootCompositionVariableStyles — <style> breakout", () => {
1522+
/**
1523+
* The payload leads with a benign `<` before its `</style`, so escaping or
1524+
* stripping only the first match does not pass: that pins the `/g` flag rather
1525+
* than merely "something ran". A lone `<` in a value (`a < b`) is the common case.
1526+
*/
1527+
const BREAKOUT = "x<y</style><script>window.__pwned=1</script><style>";
1528+
1529+
/** Emit into a document, serialize it the way the compilers do, then re-parse. */
1530+
function scriptsAfterRoundTrip(
1531+
variablesByComp: Record<string, Record<string, unknown>>,
1532+
body = "x",
1533+
) {
1534+
const { document } = parseHTML(`<!doctype html><html><head></head><body>${body}</body></html>`);
1535+
emitRootCompositionVariableStyles(document, variablesByComp);
1536+
const { document: reparsed } = parseHTML(document.toString());
1537+
return {
1538+
scripts: [...reparsed.querySelectorAll("script")].map((s) => s.textContent ?? ""),
1539+
css: [...reparsed.querySelectorAll("style")].map((s) => s.textContent ?? "").join("\n"),
1540+
reparsed,
1541+
};
1542+
}
1543+
1544+
it("does not let a variable VALUE close the style element", () => {
1545+
const { scripts } = scriptsAfterRoundTrip({ "comp-a": { brand: BREAKOUT } });
1546+
expect(scripts).toEqual([]);
1547+
});
1548+
1549+
it("does not let a COMP ID close the style element through the generated selector", () => {
1550+
// The comp id reaches the stylesheet as an attribute selector, which is escaped
1551+
// for selector-string syntax but says nothing about element termination.
1552+
const { scripts, css } = scriptsAfterRoundTrip(
1553+
{ [`comp-a${BREAKOUT}`]: { brand: "#fff" } },
1554+
'<div data-composition-id="comp-a"></div>',
1555+
);
1556+
expect(scripts).toEqual([]);
1557+
expect(css).not.toContain("</style");
1558+
});
1559+
1560+
it("strips the characters that smuggle a sibling rule, matching the runtime", () => {
1561+
// `sanitizeCssValue` is the runtime contract for a scalar folded into
1562+
// `background: var(--x)`; the compile path has to reach the same result, or a
1563+
// rendered MP4 diverges from the preview it was approved from.
1564+
const smuggle = "red; } body { background-image: url(//evil?data=1) } x { y:z";
1565+
const { css } = scriptsAfterRoundTrip({ "comp-a": { brand: smuggle } });
1566+
1567+
expect(css).not.toContain("body {");
1568+
// One rule, one declaration: with no `;{}` left in the value there is nothing to
1569+
// close the declaration with, so no sibling rule can be opened.
1570+
expect(css.match(/\}/g) ?? []).toHaveLength(1);
1571+
expect(css).toContain(`--brand: ${sanitizeCssValue(smuggle)};`);
1572+
});
1573+
1574+
it("strips '<' from a value the way the runtime does", () => {
1575+
const { css, scripts } = scriptsAfterRoundTrip({ "comp-a": { brand: "a<b" } });
1576+
expect(scripts).toEqual([]);
1577+
expect(css).not.toContain("a<b");
1578+
expect(css).toContain("ab");
1579+
});
1580+
1581+
it("leaves values without '<' untouched", () => {
1582+
const { css } = scriptsAfterRoundTrip({ "comp-a": { brand: "#ff0066" } });
1583+
expect(css).toContain("#ff0066");
1584+
});
1585+
});

0 commit comments

Comments
 (0)