fix(security): scheme-check custom htmlAttributes + deny dangerous custom tags#155
Draft
DavidBabinec wants to merge 1 commit into
Draft
fix(security): scheme-check custom htmlAttributes + deny dangerous custom tags#155DavidBabinec wants to merge 1 commit into
DavidBabinec wants to merge 1 commit into
Conversation
…stom tags 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Custom
htmlAttributesonbase.link/button/image/container/textnodes were name-filtered only (event handlers,class,style, reserveddata-*).href,src,srcdoc,formaction,xlink:href, … were permitted and their values never scheme-checked; the custom-tag escape hatch also acceptediframe,base,object,script, …A low-privilege editor holding
site.content.edit(including the Client role) — or a merely-imported HTML file — could store:href="javascript:…"(shadowing a module's own checked href — first duplicate href wins), or<iframe srcdoc="<script>…">(zero-click).Because
htmlAttributesis an object it bypassedescapeProps/sanitizeNodeProps, so the payload survived persistence and publish. It executed for published-page visitors and — critically — inside the admin editor canvas, which renders these trusted modules same-origin as/adminwith noscript-srcCSP. When a higher-privileged Owner/Admin opened a page authored by a low-trust Client, the script ran in the admin origin → session/credential theft and privilege escalation.Fix (single chokepoint + tag denylist)
All four emit/collect paths already funnel through one name gate; I extended that into a value-aware gate and applied it everywhere:
sanitizeRenderableHtmlAttribute(name, value)in@core/htmlAttributes— rejects non-renderable names, the raw-HTML sinksrcdoc, and any value carrying a dangerous URL scheme (javascript:/vbscript:/data:) via the existingisSafeUrl. Value-level scheme checking covers every URL attribute without enumerating them; ordinary text (titles, ARIA labels,viewBox) passes unchanged.normalizeHtmlAttributes), the<body>emit (bodyHtmlAttributes), and the HTML-import harvest (collectHtmlAttributes) through it.resolveHtmlTagcustom-tag escape hatch now denies dangerous elements (script,iframe,frame,frameset,object,embed,applet,base,link,meta,style) → coerced todiv.Not affected:
base.videoemits its own trusted<iframe>via its modulehtmlTag(returns'iframe'directly), notresolveHtmlTag, so video/YouTube embeds keep working.Impact
Verification
bun test src/__tests__/security/htmlAttributesXss.test.ts— 17 pass (srcdoc,javascript:/data:/vbscript:values, and dangerous custom tags dropped; safe attrs/URLs/tags preserved; publisher emit verified end-to-end).bun testacross publisher, htmlImport, base-modules, siteImport, htmlAttributes panels — 916 pass, 0 fail.bunx tsc -bclean;eslinton touched files clean;no-core-barrel-deep-importsgate passes (new@core/htmlAttributes→@core/html-sanitizeleaf import is allowed).icon-catalog-integrity) confirmed failing on the untouched base; not part of this change.🤖 Generated with Claude Code