Skip to content

fix(core): escape < in compiler-emitted composition variable CSS - #3072

Open
vanceingalls wants to merge 1 commit into
08-06-fix_core_escape_in_the_compiler-emitted_variables_scriptfrom
08-06-fix_core_escape_in_compiler-emitted_composition_variable_css
Open

fix(core): escape < in compiler-emitted composition variable CSS#3072
vanceingalls wants to merge 1 commit into
08-06-fix_core_escape_in_the_compiler-emitted_variables_scriptfrom
08-06-fix_core_escape_in_compiler-emitted_composition_variable_css

Conversation

@vanceingalls

Copy link
Copy Markdown
Collaborator

Stacked on #3071.

Same class as #3071, the other emit site. Composition variable values are written as CSS declarations inside a <style> element. <style> is also a raw-text element, so serialization leaves its content unescaped and the tokenizer closes it at the first </style regardless of CSS string context. A value containing </style> terminated the stylesheet and the remainder was parsed as markup.

Fix

Escape < to \3c in compositionVariablesCssBlock. That is the CSS escape for <, valid in every value position — including inside an unquoted url(), whose grammar permits escape sequences — so rendering is unchanged. The trailing space is consumed as part of the escape.

Variable ids need no equivalent: cssVariableName slugifies them.

Tests

Three cases in htmlBundler.test.ts: a breakout value survives a document.toString() round-trip without producing a script element, a<b lands as the CSS escape a\\3c b, and values without < are untouched. Verified to fail without the fix.

🤖 Generated with Claude Code

Composition variable values are emitted as CSS declarations inside a
`<style>` element. `<style>` is a raw-text element, so HTML serialization
leaves its content unescaped and the tokenizer closes it at the first
`</style` regardless of CSS string context. A value containing `</style>`
therefore terminated the stylesheet and the remainder was parsed as markup.

Escape `<` to `\\3c ` in `compositionVariablesCssBlock`. That is the CSS
escape for `<`, valid in every value position — including inside an
unquoted `url()`, whose grammar permits escape sequences — so rendering is
unchanged. Variable ids need no equivalent: `cssVariableName` slugifies them.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: <style> breakout via composition variable CSS

Same class as #3071, second emit site.

The vulnerability

<style> is also a raw text element — the tokenizer closes it at </style regardless of CSS context. Variable values are written directly into CSS declarations via compositionVariablesCssBlock. A value containing </style><script>… terminates the stylesheet and injects markup.

The fix

function cssSafeVariableValue(value: string | number): string {
  return String(value).replace(/</g, "\\3c ");
}

\3c is the CSS escape for <. Valid in every CSS value position (including unquoted url() where escape sequences are permitted by grammar). The trailing space is consumed as part of the escape per the CSS spec.

What I verified

  • Variable IDs don't need escaping: cssVariableName calls slugify, which collapses to [a-z0-9-] only. Confirmed in tokenSlug.ts — no < survives slugification.
  • CSS escape correctness: \3c is the correct hex escape for < (U+003C). The trailing space is required when the next character could be a hex digit; it's always safe to include.
  • Value type guard: the if on the caller side only passes string or number values, and String(number) can't produce <. The guard is defense-in-depth for strings.
  • Rendering transparency: the browser's CSS parser resolves \3c back to <, so computed values are unchanged.
  • No other <style> embedding sites: compositionVariablesCssBlock is the only function writing user-controlled values into a <style> element. Confirmed no other sinks.

Test quality

Same round-trip pattern as #3071: emit into document → serialize → reparse → assert zero <script> elements. The CSS escape check (a<ba\3c b) and the no-op check (#ff0066 untouched) verify both the transformation and its selectivity.

Stack completeness

<script> and <style> are the only two raw text elements in HTML5 (<textarea> and <title> are escapable raw text — the serializer HTML-escapes their content). This stack covers both sinks. Within this codebase, both emit sites for user-controlled composition data are patched.

CI hasn't fully triggered yet (only WIP). No blocking concerns.

— Miga

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: <style> breakout via composition variable CSS — 🟢 LGTM

HEAD fea8fba5 · mergeable=MERGEABLE · mergeStateStatus=BLOCKED (review-required).

The fix is correct, minimal, and centralized. Second emit site of the same class as #3071 — this closes the <style> RAWTEXT breakout while #3071 closed the <script> one. Non-blocking questions on CSS-injection scope below.

The hazard

<style> is one of the two HTML5 RAWTEXT elements: the tokenizer exits only on </style (case-insensitive), regardless of CSS string / comment / url() context. compositionVariablesCssBlock interpolated attacker-controlled composition values verbatim into CSS declarations, so a value carrying </style><script>…</script><style> closed the stylesheet and everything after it was parsed as markup once the producer serialized via document.toString().

The fix — verified correct

cssSafeVariableValue(v) = String(v).replace(/</g, "\\3c ") at packages/core/src/compiler/htmlBundler.ts:1141-1143, applied at the single interpolation site (line 1153).

  • CSS escape correctness: \3c is the CSS hex escape for U+003C. The trailing space is required — without it, \3c followed by a hex-digit character (\3ca, \3c3) would extend the escape (CSS reads up to 6 hex digits) and decode to a different codepoint. Spec-correct per CSS Syntax 4.3.3.
  • Every value position: valid inside strings (content: "\3c "), unquoted url() (grammar permits escape sequences), and bare values. Browser resolves back to < — computed values unchanged.
  • Sufficient character set: for RAWTEXT <style>, < is the only character that can transition out of the element. Case doesn't matter (regex is byte-level, escaping ALL <). No other characters need coverage for the stated hazard.
  • Variable IDs safe: cssVariableNameslugify collapses to [a-z0-9-], so no < can survive on the LHS. Confirmed via reading ../tokenSlug.

Completeness — all emit sites covered

compositionVariablesCssBlock is the single choke-point. All four callers in htmlBundler.ts route through it:

  • hostScopedVariableRules (line 1251) — per-instance host values
  • rootDeclaredVariableRules (line 1262) — :root declared defaults
  • declarerVariableRules (line 1285) — per-declarer marker-scoped rules
  • pushSubCompVariableStyles (line 1310) — sub-comp declared + merged host values

Verified no other function writes composition-variable values into a <style> element. Grepped: other document.createElement("style") sites emit static rules (injectTextRenderingRule line 635), inlined authored project CSS (lines 830 / 844 — different threat model, not user-supplied variable values), or the merged compStyleChunks at line 1061 (whose variable-value contributions already flow through the fixed helper).

Runtime-side variable application (applyCssVariableselement.style.setProperty) uses CSSOM and doesn't touch the HTML tokenizer, so it's a separate class and out of scope.

Coordination with #3071 — additive, no double-encoding

  • #3071 handles the <script> emitter — value goes through JSON.stringify then a JSON-unicode << rewrite.
  • #3072 handles the <style> emitter — value goes through the CSS escape <\3c .

These are different sinks with different escapes; a single composition value doesn't pass through both, so no double-encoding regression. <textarea> and <title> (the two ESCAPABLE-RAWTEXT elements) don't apply here — their serializer HTML-escapes content, so those aren't sinks.

Test coverage — sufficient

Three cases in htmlBundler.test.ts line 1398+, exercising the actual round-trip (emitRootCompositionVariableStylesdocument.toString() → re-parse):

  1. Breakout value → zero <script> elements after re-parse.
  2. a<ba\3c b in CSS, no raw < in the reparsed stylesheet.
  3. #ff0066 (no <) → unchanged, no-op verified.

I'd have also liked one negative test with < inside an unquoted url() value to lock in the "valid in every value position" claim, but the round-trip breakout test is the load-bearing assertion and it's solid.

🟡 Clarify — CSS injection surface remains (non-blocker; scope question)

The escape is deliberately narrow (only <) — the PR body explicitly bounds it. Fine for the stored-XSS goal, but the value is still emitted verbatim aside from <, so a value like

red; } body { display: none } :root { --x: red

still injects arbitrary CSS rules: the unbalanced } at top-level of a custom-property value terminates the :root { ... } block per CSS Syntax 5.4.4 (Consume a block), letting the attacker append body { … }, @font-face { src: url("//attacker/log") } (data exfil), or absolute-positioned overlays (clickjacking / defacement). Not XSS — modern browsers can't execute JS from CSS — but a real defacement / passive-exfil surface on any public-facing compiled HTML.

Is CSS injection considered a separate concern intentionally out of scope for this stored-XSS stack? Full mitigation is non-trivial (naively escaping } / ; / \ breaks legitimate string / balanced-brace values), so a follow-up ticket is a reasonable answer — I just want to confirm it's on someone's radar and not implicitly assumed closed by this PR.

What I didn't verify

  • Whether existing composition values in production data contain legitimate literal < (e.g., content: "1 < 2"). At render they'll still resolve to < post-CSS-parse (visually unchanged), but any downstream consumer that re-serializes the stylesheet without decoding will see \3c where they used to see <. Extremely unlikely in practice.
  • Whether pacific#32319 / #32320 (FE side) or #3071 (script emitter) have any code path that reads the compiled HTML back and expects raw < in --var: ... — cross-repo dependency I didn't chase because #3071's escape target is a different element entirely.
  • CI: only WIP + Mintlify skip visible at this HEAD; full CI hadn't triggered when I looked.

Peer scan

  • miga-heygen — COMMENTED (2026-08-06T21:11:38Z) — approves-in-body, same-class validation, no blockers flagged. Confirms the RAWTEXT completeness claim (<script> and <style> are the only two; <textarea> / <title> are escapable-RAWTEXT and serializer-escaped).

Review by Rames D Jusso

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants