Skip to content

Commit 91db93a

Browse files
vanceingallsclaude
andauthored
fix(core): escape < in the compiler-emitted variables script (#3071)
* fix(core): escape `<` in the compiler-emitted variables script `<script>` is a raw-text element: HTML serialization does not escape its content, and the tokenizer ends it at the first `</script`. The statement `buildVariablesByCompScript` emits embeds composition variables via `JSON.stringify`, which escapes `"` and `\` but not `/` — so a variable value, key, or composition id containing `</script>` terminated the element early and the remainder was parsed as markup, corrupting the compiled document. Rewrite `<` to its JSON unicode escape. This is transparent to JSON.parse, so the table the runtime reads is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(core): centralize the JSON-in-script escape for every emitted literal Escaping only the variables table left the composition id exploitable through the wrapper it is emitted beside: wrapScopedCompositionScript serializes the comp id, timeline comp id, authored root id, scope-selector override, error label and two derived selector patterns with a bare JSON.stringify, and wrapInlineScriptWithErrorBoundary does the same for the composition's own source. All land in the same raw-text <script>, so any one of them could close the element and have the remainder parsed as markup. Route every literal through one jsonScriptLiteral helper instead of guarding per value — the comp id alone reaches the emitted script through four separate literals, which is how the first pass missed it. Tests cover each wrapper literal via a serialize/reparse round trip, and every payload now leads with a benign `<` ahead of its `</script`, so escaping only the first match no longer passes. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 120ea37 commit 91db93a

2 files changed

Lines changed: 172 additions & 13 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
}

0 commit comments

Comments
 (0)