Fix C1/C2: XSS via unsanitized style keys and animation attrs
C1: cssPropsToString emitted the camelToKebab'd style-object KEY unsanitized while only sanitizing the VALUE. A malicious style key containing a quote (reachable via AI update_props or deserialized saved state) could close the style="..." attribute and inject a live element. Now validates each key against a CSS property/custom-prop allowlist and drops anything that doesn't match. C2: buildDataAttrs (html-export.ts) interpolated props.animation and props.animationDelay directly into data-animation="..."/ data-animation-delay="..." with no escaping, for every exported node. Now routes both through escapeAttr. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,60 @@
|
|||||||
|
import { describe, test, expect } from 'vitest';
|
||||||
|
import { exportBodyHtml } from './html-export';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* C2: buildDataAttrs (internal to html-export.ts) previously interpolated
|
||||||
|
* `props.animation` / `props.animationDelay` directly into
|
||||||
|
* `data-animation="${...}"` / `data-animation-delay="${...}"` with no
|
||||||
|
* escaping. Runs for EVERY node (via renderNode -> buildDataAttrs ->
|
||||||
|
* injectAttrs) and is reachable via AI `update_props` + deserialized saved
|
||||||
|
* state. A malicious value like `x"><img src=y onerror=alert(1)>` breaks
|
||||||
|
* out of the attribute and injects a live element.
|
||||||
|
*/
|
||||||
|
describe('buildDataAttrs escapes animation/animationDelay (C2)', () => {
|
||||||
|
const makeState = (props: Record<string, unknown>) =>
|
||||||
|
JSON.stringify({
|
||||||
|
ROOT: {
|
||||||
|
type: { resolvedName: 'Container' },
|
||||||
|
isCanvas: true,
|
||||||
|
props: { tag: 'div', style: {}, ...props },
|
||||||
|
displayName: 'Container',
|
||||||
|
custom: {},
|
||||||
|
hidden: false,
|
||||||
|
nodes: [],
|
||||||
|
linkedNodes: {},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
test('malicious animation value is escaped -- no raw breakout, no injected <img>', () => {
|
||||||
|
const payload = 'x"><img src=y onerror=alert(1)>';
|
||||||
|
const state = makeState({ animation: payload });
|
||||||
|
const { html } = exportBodyHtml(state);
|
||||||
|
|
||||||
|
expect(html).not.toContain('<img src=y onerror=alert(1)>');
|
||||||
|
expect(html).not.toMatch(/data-animation="x">/);
|
||||||
|
// The attribute value must be escaped, so no raw `"` or `<` survives
|
||||||
|
// inside the data-animation attribute region.
|
||||||
|
expect(html).toContain('data-animation="x"><img src=y onerror=alert(1)>"');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('malicious animationDelay value is escaped -- no raw breakout', () => {
|
||||||
|
const payload = 'x"><script>alert(1)</script>';
|
||||||
|
const state = makeState({ animation: 'fade-in', animationDelay: payload });
|
||||||
|
const { html } = exportBodyHtml(state);
|
||||||
|
|
||||||
|
expect(html).not.toContain('<script>alert(1)</script>');
|
||||||
|
expect(html).toContain('data-animation-delay="x"><script>alert(1)</script>"');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('normal animation value still emits data-animation="fade-in" unchanged', () => {
|
||||||
|
const state = makeState({ animation: 'fade-in' });
|
||||||
|
const { html } = exportBodyHtml(state);
|
||||||
|
expect(html).toContain('data-animation="fade-in"');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('normal animationDelay value still emits unchanged', () => {
|
||||||
|
const state = makeState({ animation: 'fade-in', animationDelay: '0.5s' });
|
||||||
|
const { html } = exportBodyHtml(state);
|
||||||
|
expect(html).toContain('data-animation-delay="0.5s"');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,6 +1,6 @@
|
|||||||
import { componentResolver } from '../components/resolver';
|
import { componentResolver } from '../components/resolver';
|
||||||
import { cssPropsToString } from './style-helpers';
|
import { cssPropsToString } from './style-helpers';
|
||||||
import { escapeHtml } from './escape';
|
import { escapeHtml, escapeAttr } from './escape';
|
||||||
|
|
||||||
export interface ExportOptions {
|
export interface ExportOptions {
|
||||||
title?: string;
|
title?: string;
|
||||||
@@ -24,9 +24,9 @@ function buildDataAttrs(props: Record<string, any>): string {
|
|||||||
if (props.hideOnTablet) attrs += ' data-hide-tablet';
|
if (props.hideOnTablet) attrs += ' data-hide-tablet';
|
||||||
if (props.hideOnMobile) attrs += ' data-hide-mobile';
|
if (props.hideOnMobile) attrs += ' data-hide-mobile';
|
||||||
if (props.animation && props.animation !== 'none') {
|
if (props.animation && props.animation !== 'none') {
|
||||||
attrs += ` data-animation="${props.animation}"`;
|
attrs += ` data-animation="${escapeAttr(String(props.animation))}"`;
|
||||||
if (props.animationDelay && props.animationDelay !== '0') {
|
if (props.animationDelay && props.animationDelay !== '0') {
|
||||||
attrs += ` data-animation-delay="${props.animationDelay}"`;
|
attrs += ` data-animation-delay="${escapeAttr(String(props.animationDelay))}"`;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return attrs;
|
return attrs;
|
||||||
|
|||||||
@@ -54,3 +54,69 @@ describe('cssPropsToString sanitizes emitted values (A5)', () => {
|
|||||||
expect(withoutEntities).not.toContain(';');
|
expect(withoutEntities).not.toContain(';');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('cssPropsToString sanitizes emitted KEYS (C1 -- style-object key breakout)', () => {
|
||||||
|
test('a key containing a double-quote is dropped entirely, not emitted', () => {
|
||||||
|
const out = cssPropsToString({
|
||||||
|
color: 'red',
|
||||||
|
'"><img src=x onerror=alert(1)>': '1',
|
||||||
|
} as any);
|
||||||
|
expect(out).not.toContain('"><img');
|
||||||
|
expect(out).not.toContain('onerror');
|
||||||
|
expect(out).toBe('color:red');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a key containing > is dropped', () => {
|
||||||
|
const out = cssPropsToString({ 'foo>bar': '1' } as any);
|
||||||
|
expect(out).toBe('');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a key containing < is dropped', () => {
|
||||||
|
const out = cssPropsToString({ 'foo<bar': '1' } as any);
|
||||||
|
expect(out).toBe('');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a key containing a semicolon is dropped', () => {
|
||||||
|
const out = cssPropsToString({ 'foo;bar': '1' } as any);
|
||||||
|
expect(out).toBe('');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a key containing a space is dropped', () => {
|
||||||
|
const out = cssPropsToString({ 'foo bar': '1' } as any);
|
||||||
|
expect(out).toBe('');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('legit simple key "color" is preserved', () => {
|
||||||
|
const out = cssPropsToString({ color: 'red' } as any);
|
||||||
|
expect(out).toBe('color:red');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('legit camelCase key backgroundColor -> background-color is preserved', () => {
|
||||||
|
const out = cssPropsToString({ backgroundColor: 'blue' } as any);
|
||||||
|
expect(out).toBe('background-color:blue');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a CSS custom property key (--custom-prop) is preserved', () => {
|
||||||
|
const out = cssPropsToString({ '--custom-prop': '10px' } as any);
|
||||||
|
expect(out).toBe('--custom-prop:10px');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a vendor-prefixed key (-webkit-...) is preserved', () => {
|
||||||
|
const out = cssPropsToString({ WebkitBoxShadow: '0 0 1px red' } as any);
|
||||||
|
expect(out).toBe('-webkit-box-shadow:0 0 1px red');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a digits-only key is dropped', () => {
|
||||||
|
const out = cssPropsToString({ '123': '1' } as any);
|
||||||
|
expect(out).toBe('');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('other legit keys in the same object still emit even when a malicious key is dropped', () => {
|
||||||
|
const out = cssPropsToString({
|
||||||
|
color: 'red',
|
||||||
|
'"><script>alert(1)</script>': 'x',
|
||||||
|
backgroundColor: 'blue',
|
||||||
|
} as any);
|
||||||
|
expect(out).toBe('color:red;background-color:blue');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -43,11 +43,24 @@ function sanitizeCssValue(raw: string): string {
|
|||||||
return out;
|
return out;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A real CSS property name (`color`, `background-color`), vendor-prefixed
|
||||||
|
// property (`-webkit-box-shadow`), or custom property (`--custom-prop`) --
|
||||||
|
// nothing else. This is a KEY allowlist, not a value sanitizer: it exists
|
||||||
|
// solely to stop a malicious style object KEY (e.g.
|
||||||
|
// `'"><img src=x onerror=alert(1)>'`, reachable via AI `update_props` or
|
||||||
|
// deserialized saved state, which spread arbitrary keys into `p.style`)
|
||||||
|
// from being emitted unescaped into `style="${camelToKebab(k)}:${...}"` and
|
||||||
|
// closing the attribute early. Legitimate keys never contain `"`, `>`, `<`,
|
||||||
|
// `;`, whitespace, `{`, `}`, or digits-only, so this never rejects real CSS.
|
||||||
|
const VALID_CSS_KEY_RE = /^-{0,2}[a-z][a-z-]*$/;
|
||||||
|
|
||||||
export function cssPropsToString(style: CSSProperties | undefined): string {
|
export function cssPropsToString(style: CSSProperties | undefined): string {
|
||||||
if (!style) return '';
|
if (!style) return '';
|
||||||
return Object.entries(style)
|
return Object.entries(style)
|
||||||
.filter(([, v]) => v !== undefined && v !== null && v !== '')
|
.filter(([, v]) => v !== undefined && v !== null && v !== '')
|
||||||
.map(([k, v]) => `${camelToKebab(k)}:${typeof v === 'string' ? sanitizeCssValue(v) : v}`)
|
.map(([k, v]) => [camelToKebab(k), v] as const)
|
||||||
|
.filter(([k]) => VALID_CSS_KEY_RE.test(k))
|
||||||
|
.map(([k, v]) => `${k}:${typeof v === 'string' ? sanitizeCssValue(v) : v}`)
|
||||||
.join(';');
|
.join(';');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user