fix(builder): sanitize non-string style values + allowlist element tags (XSS)

Adversarial re-review found the C1 fix incomplete plus an adjacent
same-class XSS, both reachable via the AI update_props path and
deserialized saved state:

- cssPropsToString only ran sanitizeCssValue on typeof-string values, so a
  non-string style value (array/object) with a valid key skipped
  sanitization entirely and was template-coerced raw into style="...",
  e.g. { color: ['red', '"><img src=x onerror=alert(1)>'] }. Now every
  non-number value is coerced with String() and sanitized; numbers stay
  raw. sanitizeBreakoutChars also now escapes < and > (previously only ;
  and ") as defense-in-depth, since values can reach it from non-string
  sources.

- props.tag (Container) and props.level (Heading) were interpolated raw
  into the tag position of exported HTML (`<${tag}`, `<${level}`) with no
  runtime validation, letting a malicious value break out of the tag
  entirely. Both are now allowlisted/clamped against their known-safe sets
  (div/section/article/header/footer/main; h1-h6), falling back to
  div/h2. Applied in Container's live render + toHtml, Heading's live
  render + toHtml, and the typeName==='div' fallback branch in
  html-export.ts's renderNode (hit for unresolved/legacy node types).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-07-12 17:44:59 -07:00
parent e12fb89ada
commit 7ba91d9829
8 changed files with 195 additions and 10 deletions
@@ -0,0 +1,40 @@
import { describe, test, expect } from 'vitest';
import { Heading } from './Heading';
const toHtml = (Heading as any).toHtml;
describe('Heading.toHtml level allowlist (adversarial re-review, same class as C1)', () => {
test('a malicious level value clamps to h2 -- no injected <img>, no broken-out tag', () => {
const { html } = toHtml({ text: 'x', level: 'h2><img src=x onerror=alert(1)' }, '');
expect(html).not.toContain('<img');
expect(html).not.toContain('onerror');
expect(html.startsWith('<h2')).toBe(true);
expect(html.endsWith('</h2>')).toBe(true);
});
test('a numeric out-of-range level (99) clamps to h2', () => {
const { html } = toHtml({ text: 'x', level: 99 as any }, '');
expect(html.startsWith('<h2')).toBe(true);
expect(html.endsWith('</h2>')).toBe(true);
});
test('a non-heading string level clamps to h2', () => {
const { html } = toHtml({ text: 'x', level: 'script' as any }, '');
expect(html.startsWith('<h2')).toBe(true);
expect(html).not.toContain('<script');
});
test('a normal valid level (h4) still emits <h4', () => {
const { html } = toHtml({ text: 'x', level: 'h4' }, '');
expect(html).toContain('<h4');
expect(html).toContain('</h4>');
});
test('all valid levels h1-h6 still work', () => {
for (const level of ['h1', 'h2', 'h3', 'h4', 'h5', 'h6']) {
const { html } = toHtml({ text: 'x', level }, '');
expect(html.startsWith(`<${level}`)).toBe(true);
expect(html.endsWith(`</${level}>`)).toBe(true);
}
});
});
+13 -2
View File
@@ -4,6 +4,16 @@ import { cssPropsToString } from '../../utils/style-helpers';
type HeadingLevel = 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6';
// `level` is settable via the AI `update_props` path and from deserialized
// saved state -- neither type-checked at runtime -- and is interpolated
// directly into the tag position (`React.createElement(level, ...)` /
// `<${tag}` in `toHtml`). A malicious value like `h2><img src=x
// onerror=alert(1)` (or a non-h1-6 string) must never reach that position
// unchecked. Anything not in this allowlist clamps to `'h2'`.
const ALLOWED_HEADING_LEVELS = ['h1', 'h2', 'h3', 'h4', 'h5', 'h6'] as const;
const sanitizeHeadingLevel = (level: unknown): HeadingLevel =>
(ALLOWED_HEADING_LEVELS as readonly unknown[]).includes(level) ? (level as HeadingLevel) : 'h2';
interface HeadingProps {
text?: string;
level?: HeadingLevel;
@@ -30,6 +40,7 @@ export const Heading: UserComponent<HeadingProps> = ({
selected: node.events.selected,
}));
const safeLevel = sanitizeHeadingLevel(level);
const elRef = useRef<HTMLElement | null>(null);
const editedTextRef = useRef<string | null>(null);
@@ -59,7 +70,7 @@ export const Heading: UserComponent<HeadingProps> = ({
}
}, [text, selected]);
return React.createElement(level, {
return React.createElement(safeLevel, {
ref: (ref: HTMLElement | null): void => {
elRef.current = ref;
if (ref) connect(drag(ref));
@@ -98,7 +109,7 @@ Heading.craft = {
};
(Heading as any).toHtml = (props: HeadingProps, _childrenHtml: string) => {
const tag = props.level || 'h2';
const tag = sanitizeHeadingLevel(props.level);
const safeText = (props.text || '').replace(/</g, '&lt;').replace(/>/g, '&gt;');
const styleStr = cssPropsToString(props.style);
return { html: `<${tag}${styleStr ? ` style="${styleStr}"` : ''}>${safeText}</${tag}>` };
@@ -33,3 +33,33 @@ describe('Container.toHtml cssId/cssClass', () => {
expect(html).toContain('id="anchor-id"');
});
});
describe('Container.toHtml tag allowlist (adversarial re-review, same class as C1)', () => {
test('a malicious tag value falls back to div -- no injected <img>, no broken-out attrs', () => {
const { html } = toHtml({ tag: 'div><img src=x onerror=alert(1)' }, 'child');
expect(html).not.toContain('<img');
expect(html).not.toContain('onerror');
expect(html.startsWith('<div')).toBe(true);
expect(html.endsWith('</div>')).toBe(true);
});
test('a tag value outside the known-safe set falls back to div', () => {
const { html } = toHtml({ tag: 'script' }, 'child');
expect(html.startsWith('<div')).toBe(true);
expect(html).not.toContain('<script');
});
test('a valid tag (section) still emits <section', () => {
const { html } = toHtml({ tag: 'section' }, 'child');
expect(html).toContain('<section');
expect(html).toContain('</section>');
});
test('all other allowlisted tags still work', () => {
for (const tag of ['div', 'article', 'header', 'footer', 'main']) {
const { html } = toHtml({ tag }, 'child');
expect(html.startsWith(`<${tag}`)).toBe(true);
expect(html.endsWith(`</${tag}>`)).toBe(true);
}
});
});
+16 -2
View File
@@ -3,6 +3,19 @@ import { useNode, UserComponent } from '@craftjs/core';
import { cssPropsToString } from '../../utils/style-helpers';
import { escapeAttr } from '../../utils/escape';
// The only tag names Container actually supports (matches the TS union
// below and the `tag` default in `.craft.props`). `tag` is settable via the
// AI `update_props` path and from deserialized saved state -- neither is
// type-checked at runtime -- so a malicious value like
// `div><img src=x onerror=alert(1)` must never reach the `<${tag}` template
// position in `toHtml`/the live render. Anything not in this allowlist
// falls back to `'div'`.
const ALLOWED_CONTAINER_TAGS = ['div', 'section', 'article', 'header', 'footer', 'main'] as const;
export type ContainerTag = (typeof ALLOWED_CONTAINER_TAGS)[number];
export const sanitizeContainerTag = (tag: unknown): ContainerTag =>
(ALLOWED_CONTAINER_TAGS as readonly unknown[]).includes(tag) ? (tag as ContainerTag) : 'div';
interface ContainerProps {
style?: CSSProperties;
tag?: 'div' | 'section' | 'article' | 'header' | 'footer' | 'main';
@@ -42,6 +55,7 @@ export const Container: UserComponent<ContainerProps> = ({
}) => {
const { connectors: { connect, drag } } = useNode();
const safeTag = sanitizeContainerTag(tag);
const needsBoxedWrapper = contentWidth === 'boxed';
const flexStyles = flexAlignFromTextAlign(style.textAlign);
@@ -58,7 +72,7 @@ export const Container: UserComponent<ContainerProps> = ({
const idValue = cssId || anchorId || undefined;
const el = React.createElement(
tag,
safeTag,
{
ref: (ref: HTMLElement | null): void => { if (ref) connect(drag(ref)); },
style: outerStyle,
@@ -97,7 +111,7 @@ Container.craft = {
/* ---------- HTML export ---------- */
(Container as any).toHtml = (props: ContainerProps, childrenHtml: string) => {
const tag = props.tag || 'div';
const tag = sanitizeContainerTag(props.tag);
const isBoxed = props.contentWidth === 'boxed';
const flexStyles = flexAlignFromTextAlign(props.style?.textAlign);