shadowdao
7ba91d9829
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>
83 lines
4.1 KiB
TypeScript
83 lines
4.1 KiB
TypeScript
import { CSSProperties } from 'react';
|
|
import { escapeAttr, safeUrl } from './escape';
|
|
|
|
const camelToKebab = (str: string): string =>
|
|
str.replace(/[A-Z]/g, (m) => '-' + m.toLowerCase());
|
|
|
|
const URL_RE = /url\(\s*(['"]?)([\s\S]*?)\1\s*\)/gi;
|
|
|
|
// Outside of a url(...) reference, a `;` is never legitimate (declarations
|
|
// are separated by it) -- stray semicolons are how a breakout injects a
|
|
// second property -- and a raw `"` would close the `style="..."` attribute
|
|
// early. `<`/`>` are escaped too as defense-in-depth: they're inert inside a
|
|
// properly quote-terminated `style="..."` attribute, but a value can reach
|
|
// this function from a non-string source (array/object coerced via
|
|
// `String(v)`, see `cssPropsToString` below) so we don't want to rely solely
|
|
// on the outer quote holding. Inside url('...') the content has already been
|
|
// made safe via escapeAttr(safeUrl(...)), including any `;` required by
|
|
// data-URI syntax (`data:<mime>;base64,<payload>`), so this must never be
|
|
// applied there.
|
|
const sanitizeBreakoutChars = (s: string): string =>
|
|
s.replace(/;/g, '').replace(/"/g, '"').replace(/</g, '<').replace(/>/g, '>');
|
|
|
|
/**
|
|
* Sanitizes a single CSS declaration value so it can never terminate the
|
|
* `style="..."` attribute early, inject an extra declaration via a stray
|
|
* `;`, or smuggle a `javascript:`/`vbscript:`/`data:text/html` URL through
|
|
* a `url(...)` reference. Legitimate multi-part values (box-shadow,
|
|
* gradients, etc.) that contain none of these characters pass through
|
|
* unchanged, and legitimate `;`-containing data-URIs inside url(...) are
|
|
* preserved intact.
|
|
*/
|
|
function sanitizeCssValue(raw: string): string {
|
|
let out = '';
|
|
let lastIndex = 0;
|
|
URL_RE.lastIndex = 0;
|
|
let m: RegExpExecArray | null;
|
|
while ((m = URL_RE.exec(raw)) !== null) {
|
|
// Sanitize breakout characters only in the segment before this url(...)
|
|
// reference -- never inside the reference itself.
|
|
out += sanitizeBreakoutChars(raw.slice(lastIndex, m.index));
|
|
// Neutralize the url(...) reference: validate/strip the scheme and
|
|
// re-wrap in single quotes with the contents escaped for attribute
|
|
// safety. This is already fully safe, `;` and all.
|
|
const inner = m[2];
|
|
out += `url('${escapeAttr(safeUrl(inner.trim()))}')`;
|
|
lastIndex = URL_RE.lastIndex;
|
|
}
|
|
out += sanitizeBreakoutChars(raw.slice(lastIndex));
|
|
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 {
|
|
if (!style) return '';
|
|
return Object.entries(style)
|
|
.filter(([, v]) => v !== undefined && v !== null && v !== '')
|
|
.map(([k, v]) => [camelToKebab(k), v] as const)
|
|
.filter(([k]) => VALID_CSS_KEY_RE.test(k))
|
|
// Only a genuine `number` is safe to interpolate raw (numbers can never
|
|
// contain a `"`/`<`/`>`/`;` breakout character). Every other type --
|
|
// string, array, object, etc. -- must be coerced to a string and run
|
|
// through `sanitizeCssValue`. Without this, a non-string value (e.g. an
|
|
// array like `['red', '"><img src=x onerror=alert(1)>']`) skips
|
|
// sanitization entirely and is template-coerced (`${v}`) raw into the
|
|
// `style="..."` attribute, breaking out via the un-escaped `"`.
|
|
.map(([k, v]) => `${k}:${typeof v === 'number' ? v : sanitizeCssValue(String(v))}`)
|
|
.join(';');
|
|
}
|
|
|
|
export function mergeStyles(...styles: (CSSProperties | undefined)[]): CSSProperties {
|
|
return Object.assign({}, ...styles.filter(Boolean));
|
|
}
|