diff --git a/docs/content/4.plugins/1.built-in/security.md b/docs/content/4.plugins/1.built-in/security.md index 8b24bf6d..1de68551 100644 --- a/docs/content/4.plugins/1.built-in/security.md +++ b/docs/content/4.plugins/1.built-in/security.md @@ -96,6 +96,13 @@ Attributes that can be abused regardless of value are always stripped: |---|---| | `srcdoc` | Can contain arbitrary HTML | | `formaction` | Can redirect form submissions | +| `innerHTML` | Injects raw HTML through framework renderers | +| `dangerouslySetInnerHTML` | Injects raw HTML through framework renderers | +| `textContent` | Overwrites an element's children | + +::note +Framework renderers (Vue, React, Svelte, Angular) never forward `innerHTML`, `dangerouslySetInnerHTML`, or `textContent` from document attributes, even without this plugin. Raw HTML has its own explicit path through the default `html` plugin. +:: ### Protocol blocking @@ -103,6 +110,8 @@ Attributes that can be abused regardless of value are always stripped: `javascript:` · `vbscript:` · `data:text/html` · `data:text/javascript` · `data:text/vbscript` · `data:text/css` · `data:text/plain` · `data:text/xml` +The same check applies to `:href` and `:src` bindings twice: on the JSON-decoded value at parse time, and again on the resolved value at render time, so bindings cannot smuggle an unsafe URL through frontmatter or other data sources. + ::code-group ```html [Input] @@ -177,6 +186,8 @@ security({ }) ``` +The `as` prop (which makes framework renderers resolve a different component than the element's own tag) is held to the same filters: an `as` value naming a blocked or not-allowed tag is stripped, and the element falls back to its own tag. + ### `tagFallback` Defines the replacement strategy for tags that are filtered out because they are not present in the `allowedTags` (whitelist) or present in the `blockedTags` (blacklist). @@ -212,6 +223,8 @@ The hard-coded unsafe protocols (`javascript:`, `vbscript:`, `data:text/*`) are Restricts which URLs are allowed in `href` attributes. Relative URLs (starting with `/`, `#`, etc.) are always allowed regardless of this setting. +Prefixes compare by parsed origin plus a path-segment boundary, not by raw string matching: `https://myapp.com` allows `https://myapp.com/docs` but never a lookalike host such as `https://myapp.com.evil.com`. Scheme-relative URLs (`//evil.com/page`) resolve to an absolute URL and go through the same checks. + When a URL does not match any prefix and `defaultOrigin` is set, the URL is rewritten instead of stripped. ```typescript diff --git a/packages/comark-angular/src/components/markdown-node.component.ts b/packages/comark-angular/src/components/markdown-node.component.ts index daabd835..b6401f59 100644 --- a/packages/comark-angular/src/components/markdown-node.component.ts +++ b/packages/comark-angular/src/components/markdown-node.component.ts @@ -194,9 +194,9 @@ export class MarkdownNode implements OnChanges { const el = this.renderer.createElement(tag) this.applyAttributes(el, attrs) - if (attrs['innerHTML'] != null) { - el.innerHTML = attrs['innerHTML'] - } else if (!VOID_ELEMENTS.has(tag)) { + // `innerHTML` from document attributes is never applied — resolveAttributes + // drops DOM sink props, and raw HTML has its own explicit parse path. + if (!VOID_ELEMENTS.has(tag)) { this.renderChildren(el, children, childrenRenderData) } diff --git a/packages/comark-vue/test/sink-props.test.ts b/packages/comark-vue/test/sink-props.test.ts new file mode 100644 index 00000000..c3c765d6 --- /dev/null +++ b/packages/comark-vue/test/sink-props.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest' +import { createSSRApp, h } from 'vue' +import { renderToString } from '@vue/server-renderer' +import { parseMarkdown } from 'comark' +import { MarkdownDocument } from '../src/components/MarkdownDocument.ts' + +function renderDocument(document: unknown) { + const app = createSSRApp({ + setup() { + return () => h(MarkdownDocument, { value: document }) + }, + }) + return renderToString(app as any) +} + +describe('HTML sink props', () => { + it('never forwards markdown-authored innerHTML to h()', async () => { + const document = await parseMarkdown('::div{innerHTML=""}\n::') + const html = await renderDocument(document) + expect(html).not.toContain('') + expect(html).not.toContain('onerror') + }) + + it('drops textContent and dangerouslySetInnerHTML props', async () => { + const document = await parseMarkdown(':span[safe]{textContent="overlay"}') + const html = await renderDocument(document) + expect(html).toContain('safe') + expect(html).not.toContain('overlay') + }) +}) diff --git a/packages/comark/src/internal/props-validation.ts b/packages/comark/src/internal/props-validation.ts index f83d261d..e83ec5c5 100644 --- a/packages/comark/src/internal/props-validation.ts +++ b/packages/comark/src/internal/props-validation.ts @@ -7,7 +7,12 @@ export const REJECTED_PROP = Symbol('comark:rejected-prop') export const unsafeTags = ['object'] -export const unsafeAttributes = ['srcdoc', 'formaction'] +// `innerHTML` / `dangerouslySetInnerHTML` / `textContent` are DOM sinks that +// turn a string prop into raw markup or overwrite an element's children. +// Framework renderers receive resolved props verbatim, so markdown-authored +// values would otherwise bypass tag filtering (raw HTML has its own explicit +// path via the `html` plugin and does not need these). +export const unsafeAttributes = ['srcdoc', 'formaction', 'innerhtml', 'dangerouslysetinnerhtml', 'textcontent'] export const unsafeLinkPrefix = [ 'javascript:', @@ -40,6 +45,47 @@ function rewriteToDefaultOrigin(urlStr: string, defaultOrigin: string): string { } } +// Named entities relevant to URL smuggling — the full HTML5 table is huge, +// but these are the ones that can hide a scheme or whitespace inside it. +const NAMED_ENTITIES: Record = { + amp: '&', + lt: '<', + gt: '>', + quot: '"', + apos: "'", + colon: ':', + sol: '/', + bsol: '\\', + Tab: '\t', + NewLine: '\n', +} + +/** + * Decode HTML entities until stable. Browsers entity-decode attribute values + * before navigating, so validation must inspect the decoded form — stripping + * entities instead (the previous behavior) let `javascript:alert(1)` + * through as a "relative" URL. Repeating the pass catches nested encodings + * like `&#58;`. + */ +function decodeHtmlEntities(value: string): string { + let result = value + for (let pass = 0; pass < 10; pass++) { + const decoded = result + .replace(/&#x([0-9a-f]+);?/gi, (match, hex) => { + const code = Number.parseInt(hex, 16) + return code <= 0x10ffff ? String.fromCodePoint(code) : match + }) + .replace(/&#(\d+);?/g, (match, dec) => { + const code = Number.parseInt(dec, 10) + return code <= 0x10ffff ? String.fromCodePoint(code) : match + }) + .replace(/&([a-z]+);?/gi, (match, name) => NAMED_ENTITIES[name] ?? match) + if (decoded === result) break + result = decoded + } + return result +} + function validateUrl( value: string, mode: 'link' | 'image', @@ -53,19 +99,39 @@ function validateUrl( allowDataImages = true, } = options - const decodedUrl = decodeURIComponent(value) - const urlSanitized = decodedUrl - .replace(/&#x([0-9a-f]+);?/gi, '') - .replace(/&#(\d+);?/g, '') - .replace(/&[a-z]+;?/gi, '') + let decodedUrl: string + try { + decodedUrl = decodeURIComponent(value) + } catch { + // Malformed percent-encoding — inspect the raw value instead of throwing + decodedUrl = value + } + const urlSanitized = decodeHtmlEntities(decodedUrl) + + // Dummy origin used to unmask scheme-relative (//host) and backslash + // (\\host) URLs: resolved against it, those land on the attacker's origin, + // while genuinely relative paths stay on the dummy origin. + const DUMMY_BASE = 'http://comark.invalid' let url: URL try { // Parse without a base — throws for relative URLs, succeeds for absolute url = new URL(urlSanitized) } catch { - // Relative URLs are always allowed - return value + let resolved: URL + try { + resolved = new URL(urlSanitized, DUMMY_BASE) + } catch { + // Unparseable even with a base — treat as relative + return value + } + if (resolved.origin === DUMMY_BASE) { + // Genuinely relative URLs are always allowed + return value + } + // Scheme-relative/backslash form — check it as the absolute URL it + // resolves to in the browser + url = resolved } // Block known-unsafe protocols — hard floor, not overrideable by options @@ -89,8 +155,7 @@ function validateUrl( // Check allowed URL prefixes const allowedPrefixes = mode === 'link' ? allowedLinkPrefixes : allowedImagePrefixes if (!allowedPrefixes.includes('*')) { - const href = url.href.toLowerCase() - const matchesPrefix = allowedPrefixes.some((prefix) => href.startsWith(prefix.toLowerCase())) + const matchesPrefix = allowedPrefixes.some((prefix) => matchesAllowedPrefix(url, prefix)) if (!matchesPrefix) { if (defaultOrigin) { return rewriteToDefaultOrigin(urlSanitized, defaultOrigin) @@ -102,7 +167,56 @@ function validateUrl( return value } +/** + * Whether `url` matches an allowed prefix. Absolute-URL prefixes compare by + * parsed origin plus a path-segment boundary, so a lookalike host such as + * `https://myapp.com.evil.com` never matches the prefix `https://myapp.com`. + * Non-URL prefixes (unusual) fall back to a raw string prefix match. + */ +function matchesAllowedPrefix(url: URL, prefix: string): boolean { + const normalized = prefix.toLowerCase() + if (!normalized.includes('://')) { + return url.href.toLowerCase().startsWith(normalized) + } + let prefixUrl: URL + try { + prefixUrl = new URL(normalized) + } catch { + return url.href.toLowerCase().startsWith(normalized) + } + if (url.origin.toLowerCase() !== prefixUrl.origin.toLowerCase()) return false + const prefixPath = prefixUrl.pathname + if (prefixPath === '/') return true + const path = url.pathname.toLowerCase() + return path === prefixPath || path.startsWith(prefixPath.endsWith('/') ? prefixPath : `${prefixPath}/`) +} + +/** + * Hard-floor check: does this string resolve to a known-unsafe URL scheme + * (`javascript:`, `data:text/html`, …)? Applied to binding-resolved values at + * render time so dot-path data (frontmatter/meta/data) cannot smuggle an + * unsafe URL past parse-time validation. Relative URLs are never unsafe here. + */ +export function isUnsafeUrlValue(value: string): boolean { + let decoded = value + try { + decoded = decodeURIComponent(value) + } catch { + // Malformed percent-encoding — inspect the raw value + } + const sanitized = decodeHtmlEntities(decoded) + + let url: URL + try { + url = new URL(sanitized) + } catch { + return false + } + return unsafeLinkPrefix.some((prefix) => url.href.toLowerCase().startsWith(prefix)) +} + export function validateProp(attribute: string, value: unknown, options: PropsValidationOptions = {}): unknown { + const isBinding = /^(:|v-bind:)/.test(attribute) attribute = attribute .toLowerCase() .replace(/^(:|v-bind:)/, '') @@ -112,15 +226,31 @@ export function validateProp(attribute: string, value: unknown, options: PropsVa return REJECTED_PROP } - if (attribute === 'href' || attribute === 'xlinkhref') { - // A non-string href can reach here as an array/object from the YAML + if (attribute === 'href' || attribute === 'xlinkhref' || attribute === 'src') { + // A non-string href/src can reach here as an array/object from the YAML // block-props JSON round-trip. Reject it instead of passing it through // unvalidated (#367). - return typeof value === 'string' ? validateUrl(value, 'link', options) : REJECTED_PROP - } + if (typeof value !== 'string') return REJECTED_PROP + + // Renderers JSON-decode `:binding` values before use, so validate the + // decoded form — otherwise ':href' with '"javascript:..."' (a JSON-quoted + // string) fails URL parsing and slips through as a "relative" URL. + let effective = value + if (isBinding) { + try { + const parsed: unknown = JSON.parse(value) + if (typeof parsed === 'string') effective = parsed + } catch { + // Not JSON — a dot-path binding or literal, validated as-is + } + } - if (attribute === 'src') { - return typeof value === 'string' ? validateUrl(value, 'image', options) : REJECTED_PROP + const mode = attribute === 'src' ? 'image' : 'link' + const result = validateUrl(effective, mode, options) + if (result === REJECTED_PROP) return REJECTED_PROP + // Keep the original value so bindings still resolve at render time. The + // defaultOrigin rewrite only makes sense for literal URLs. + return isBinding ? value : result } return value diff --git a/packages/comark/src/internal/stringify/attributes.ts b/packages/comark/src/internal/stringify/attributes.ts index f123881e..9c906468 100644 --- a/packages/comark/src/internal/stringify/attributes.ts +++ b/packages/comark/src/internal/stringify/attributes.ts @@ -1,5 +1,6 @@ import { stringifyYaml } from '../yaml.ts' -import { escapeHtml, get } from '../../utils/index.ts' +import { get } from '../../utils/index.ts' +import { isUnsafeUrlValue } from '../props-validation.ts' import type { NodeRenderData } from '../../types.ts' export interface ResolveAttributesOptions { @@ -18,6 +19,13 @@ export interface ResolveAttributesOptions { parseJson?: boolean } +// DOM sinks that turn a string/object prop into raw markup (`innerHTML`, +// `dangerouslySetInnerHTML`) or overwrite an element's children +// (`textContent`). Framework renderers hand resolved attributes to +// `h()`/`createElement`/spreads verbatim, so these keys are never forwarded +// from document attributes — raw HTML has its own explicit path. +const HTML_SINK_PROPS = new Set(['innerhtml', 'dangerouslysetinnerhtml', 'textcontent']) + /** * Resolve `:prefixed` attributes against the render context. * @@ -42,34 +50,55 @@ export function resolveAttributes( const value = attrs[key] const isBinding = key.charCodeAt(0) === 58 /* ':' */ + const outKey = isBinding ? key.slice(1) : key + + if (HTML_SINK_PROPS.has(outKey.toLowerCase())) continue + + let outValue: unknown + let resultKey = key if (options.parseJson && isBinding) { // Framework mode: always strip `:` and hand components real JS values. if (typeof value === 'string') { try { - result[key.slice(1)] = JSON.parse(value) - continue + outValue = JSON.parse(value) } catch { // not JSON — fall through to dot-path lookup + outValue = get(renderData, value) } - result[key.slice(1)] = get(renderData, value) - continue + } else { + // Non-string binding value (e.g. an object literal the parser already + // decoded) — pass through with the prefix stripped. + outValue = value } - // Non-string binding value (e.g. an object literal the parser already - // decoded) — pass through with the prefix stripped. - result[key.slice(1)] = value - continue - } - - if (isBinding && typeof value === 'string') { + resultKey = outKey + } else if (isBinding && typeof value === 'string') { const resolved = get(renderData, value) if (resolved !== undefined) { - result[key.slice(1)] = resolved - continue + outValue = resolved + resultKey = outKey + } else { + outValue = value } + } else { + outValue = value } - result[key] = value + // Hard floor: a binding must never resolve href/src to an unsafe scheme + // (javascript:, data:text/html, …). Parse-time validation only sees the + // literal path, so the resolved value is checked here — even when the + // security plugin is not enabled. + const lowerOutKey = outKey.toLowerCase() + if ( + isBinding && + (lowerOutKey === 'href' || lowerOutKey === 'src' || lowerOutKey === 'xlink:href') && + typeof outValue === 'string' && + isUnsafeUrlValue(outValue) + ) { + continue + } + + result[resultKey] = outValue } return result } @@ -188,6 +217,16 @@ export function comarkAttributes(attributes: Record) { return attrs.length > 0 ? `{${attrs}}` : '' } +/** + * Escape a value for interpolation into a double-quoted HTML attribute. + * Prevents attribute breakout (`"` terminating the value early) and markup + * injection (`<`/`>` closing the tag), which would otherwise bypass + * AST-level sanitization such as `comark/plugins/security`. + */ +function escapeHtmlAttribute(value: unknown): string { + return String(value).replace(/&/g, '&').replace(/"/g, '"').replace(//g, '>') +} + // HTML attribute names must start with a letter/underscore/colon and may only // contain alphanumerics plus `_ : . -`. Anything else (quotes, spaces, …) // could break out of the attribute list, so such keys are dropped entirely. @@ -211,10 +250,10 @@ export function htmlAttributes(attributes: Record) { continue } if (typeof value === 'object' && value !== null) { - parts.push(`${key}="${escapeHtml(JSON.stringify(value))}"`) + parts.push(`${key}="${escapeHtmlAttribute(JSON.stringify(value))}"`) continue } - parts.push(`${key}="${escapeHtml(String(value))}"`) + parts.push(`${key}="${escapeHtmlAttribute(value)}"`) continue } @@ -225,11 +264,11 @@ export function htmlAttributes(attributes: Record) { if (value === false || value === null || value === undefined) continue if (typeof value === 'object') { - parts.push(`${key}="${escapeHtml(JSON.stringify(value))}"`) + parts.push(`${key}="${escapeHtmlAttribute(JSON.stringify(value))}"`) continue } - parts.push(`${key}="${escapeHtml(String(value))}"`) + parts.push(`${key}="${escapeHtmlAttribute(value)}"`) } return parts.join(' ') } diff --git a/packages/comark/src/plugins/security.ts b/packages/comark/src/plugins/security.ts index e7ccdfd5..924782aa 100644 --- a/packages/comark/src/plugins/security.ts +++ b/packages/comark/src/plugins/security.ts @@ -69,6 +69,18 @@ export default defineComarkPlugin((options: SecurityOptions = {}) => { return false } + // The `as` prop makes renderers resolve a different component than + // the element's own tag — hold it to the same tag filters, otherwise + // `[x]{as="AdminPanel"}` bypasses allowedTags/blockedTags. + const asValue = element[1].as + if (typeof asValue === 'string') { + const asTag = asValue.toLowerCase() + if (dropSet.has(asTag) || (allowSet.size > 0 && !allowSet.has(asTag))) { + console.warn(`[comark/plugins/security] removing unsafe attribute: as="${asValue}"`) + delete element[1].as + } + } + const keys = Object.keys(element[1]) /** diff --git a/packages/comark/test/plugins/security.test.ts b/packages/comark/test/plugins/security.test.ts index 5c5b99fd..e28ed0f5 100644 --- a/packages/comark/test/plugins/security.test.ts +++ b/packages/comark/test/plugins/security.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest' import { parseMarkdown } from '../../src/parse' import security from '../../src/plugins/security' import { textContent } from '../../src/utils/index.ts' -import { renderMarkdown } from 'comark/render' +import { render, renderMarkdown } from 'comark/render' import type { ElementNode, Node, MarkdownDocument } from '../../src/types' const parseWithSecurity = (md: string, options: Parameters[0] = {}) => @@ -273,6 +273,69 @@ click expect(anchor).toBeDefined() expect(anchor![1].href ?? anchor![1]['v-bind:href']).toBeUndefined() }) + + it('rejects JSON-quoted javascript: URLs in :href bindings', async () => { + const tree = await parseWithSecurity( + `\ +::a{:href='"javascript:alert(1)"'} +click +:: +`.trim() + ) + + const anchor = collectElements(tree.nodes).find((element) => element[0] === 'a') + expect(anchor).toBeDefined() + expect(anchor![1].href).toBeUndefined() + expect(anchor![1][':href']).toBeUndefined() + }) + + it('keeps JSON-quoted safe URLs in :href bindings', async () => { + const tree = await parseWithSecurity( + `\ +::a{:href='"https://example.com"'} +click +:: +`.trim() + ) + + const anchor = collectElements(tree.nodes).find((element) => element[0] === 'a') + expect(anchor).toBeDefined() + expect(anchor![1][':href']).toBe('"https://example.com"') + }) + + it('blocks javascript: URLs smuggled through frontmatter bindings at render time', async () => { + const md = `--- +home: javascript:alert(1) +--- + +[Home](placeholder){:href="frontmatter.home"}` + + const tree = await parseWithSecurity(md) + // Parse-time validation only sees the literal dot-path… + const anchor = collectElements(tree.nodes).find((element) => element[0] === 'a') + expect(anchor).toBeDefined() + // …but the render-time hard floor drops the resolved unsafe URL. + const html = await render(tree, { format: 'text/html', blockSeparator: '\n' }) + expect(html).not.toContain('javascript:') + }) + + it('strips framework HTML sink props from components', async () => { + const tree = await parseWithSecurity( + `\ +::div{innerHTML=""} +:: + +:span{:dangerouslySetInnerHTML='{"__html":""}'} +`.trim() + ) + + const div = collectElements(tree.nodes).find((element) => element[0] === 'div') + const span = collectElements(tree.nodes).find((element) => element[0] === 'span') + expect(div).toBeDefined() + expect(div![1].innerHTML).toBeUndefined() + expect(span).toBeDefined() + expect(span![1][':dangerouslySetInnerHTML']).toBeUndefined() + }) }) describe('security plugin — blockedTags', () => { @@ -390,6 +453,37 @@ describe('security plugin — allowedTags', () => { }) }) +describe('security plugin — as prop', () => { + it('strips as pointing at a blocked tag', async () => { + const tree = makeTree([['span', { as: 'script' }, 'x']]) + await runPlugin(tree, { blockedTags: ['script'] }) + const el = tree.nodes[0] as [string, any] + expect(el[0]).toBe('span') + expect(el[1].as).toBeUndefined() + }) + + it('strips as pointing at a tag outside allowedTags', async () => { + const tree = makeTree([['span', { as: 'AdminPanel' }, 'x']]) + await runPlugin(tree, { allowedTags: ['span'] }) + const el = tree.nodes[0] as [string, any] + expect(el[1].as).toBeUndefined() + }) + + it('keeps as when the resolved tag is allowed', async () => { + const tree = makeTree([['span', { as: 'Badge' }, 'x']]) + await runPlugin(tree, { allowedTags: ['span', 'badge'] }) + const el = tree.nodes[0] as [string, any] + expect(el[1].as).toBe('Badge') + }) + + it('keeps as when no tag filters are configured', async () => { + const tree = makeTree([['span', { as: 'Badge' }, 'x']]) + await runPlugin(tree) + const el = tree.nodes[0] as [string, any] + expect(el[1].as).toBe('Badge') + }) +}) + describe('security plugin — prop sanitization', () => { it('strips event handler props', async () => { const tree = makeTree([['div', { onclick: 'evil()', class: 'safe' }]]) diff --git a/packages/comark/test/props-validation.test.ts b/packages/comark/test/props-validation.test.ts index 038182a6..6c067de3 100644 --- a/packages/comark/test/props-validation.test.ts +++ b/packages/comark/test/props-validation.test.ts @@ -28,6 +28,20 @@ describe('validateProp', () => { it('blocks formaction', () => { expect(validateProp('formaction', 'https://evil.com')).toBe(REJECTED_PROP) }) + + it('blocks innerHTML (any case)', () => { + expect(validateProp('innerHTML', '')).toBe(REJECTED_PROP) + expect(validateProp('innerHtml', '')).toBe(REJECTED_PROP) + expect(validateProp('INNERHTML', '')).toBe(REJECTED_PROP) + }) + + it('blocks dangerouslySetInnerHTML', () => { + expect(validateProp('dangerouslySetInnerHTML', { __html: '' })).toBe(REJECTED_PROP) + }) + + it('blocks textContent', () => { + expect(validateProp('textContent', 'overlay')).toBe(REJECTED_PROP) + }) }) describe('href safety', () => { @@ -79,6 +93,27 @@ describe('validateProp', () => { expect(validateProp('href', 'data:text/vbscript,evil')).toBe(REJECTED_PROP) }) + it('blocks entity-encoded javascript: hrefs', () => { + expect(validateProp('href', 'javascript:alert(1)')).toBe(REJECTED_PROP) + expect(validateProp('href', 'javascript:alert(1)')).toBe(REJECTED_PROP) + expect(validateProp('href', 'javascript:alert(1)')).toBe(REJECTED_PROP) + }) + + it('blocks entity-encoded whitespace inside the scheme', () => { + expect(validateProp('href', 'jav ascript:alert(1)')).toBe(REJECTED_PROP) + expect(validateProp('href', 'java script:alert(1)')).toBe(REJECTED_PROP) + expect(validateProp('href', ' javascript:alert(1)')).toBe(REJECTED_PROP) + }) + + it('blocks nested-encoded javascript: hrefs', () => { + expect(validateProp('href', 'javascript&#58;alert(1)')).toBe(REJECTED_PROP) + }) + + it('allows safe URLs containing entities', () => { + expect(validateProp('href', '/search?q=a&b=2')).toBe('/search?q=a&b=2') + expect(validateProp('href', 'https://example.com/?a=1&b=2')).toBe('https://example.com/?a=1&b=2') + }) + it('blocks vbscript: hrefs', () => { expect(validateProp('href', 'vbscript:MsgBox(1)')).toBe(REJECTED_PROP) }) @@ -209,6 +244,51 @@ describe('validateProp', () => { 'https://any.com/img.png' ) }) + + it('blocks scheme-relative URLs pointing at other hosts', () => { + expect(validateProp('href', '//evil.com/p', { allowedLinkPrefixes: ['https://myapp.com'] })).toBe(REJECTED_PROP) + }) + + it('blocks backslash-relative URLs pointing at other hosts', () => { + expect(validateProp('href', '\\\\evil.com/p', { allowedLinkPrefixes: ['https://myapp.com'] })).toBe(REJECTED_PROP) + }) + + it('blocks lookalike hosts that share a string prefix', () => { + expect(validateProp('href', 'https://myapp.com.evil.com/p', { allowedLinkPrefixes: ['https://myapp.com'] })).toBe( + REJECTED_PROP + ) + }) + + it('allows subpaths of an allowed origin', () => { + expect(validateProp('href', 'https://myapp.com/docs/page', { allowedLinkPrefixes: ['https://myapp.com'] })).toBe( + 'https://myapp.com/docs/page' + ) + }) + + it('allows lookalike-host URLs with the default policy', () => { + expect(validateProp('href', 'https://myapp.com.evil.com/p')).toBe('https://myapp.com.evil.com/p') + expect(validateProp('href', '//evil.com/p')).toBe('//evil.com/p') + }) + }) + + describe('binding values', () => { + it('validates the JSON-decoded form of :href bindings', () => { + expect(validateProp(':href', '"javascript:alert(1)"')).toBe(REJECTED_PROP) + expect(validateProp('v-bind:href', '"javascript:alert(1)"')).toBe(REJECTED_PROP) + expect(validateProp(':src', '"data:text/html,"')).toBe(REJECTED_PROP) + }) + + it('keeps the original binding string when the decoded URL is safe', () => { + expect(validateProp(':href', '"https://example.com"')).toBe('"https://example.com"') + }) + + it('passes dot-path bindings through unchanged', () => { + expect(validateProp(':href', 'frontmatter.home')).toBe('frontmatter.home') + }) + + it('does not JSON-decode plain (non-binding) hrefs', () => { + expect(validateProp('href', '"https://example.com"')).toBe('"https://example.com"') + }) }) describe('allowedImagePrefixes', () => { @@ -230,6 +310,12 @@ describe('validateProp', () => { ) }) + it('blocks scheme-relative src pointing at other hosts', () => { + expect( + validateProp('src', '//tracker.evil.com/px.gif', { allowedImagePrefixes: ['https://cdn.myapp.com'] }) + ).toBe(REJECTED_PROP) + }) + it('rewrites disallowed src to defaultOrigin when provided', () => { const result = validateProp('src', 'https://evil.com/tracker.gif', { allowedImagePrefixes: ['https://cdn.myapp.com'], diff --git a/packages/comark/test/resolve-attributes.test.ts b/packages/comark/test/resolve-attributes.test.ts index e7960529..bab8b0b2 100644 --- a/packages/comark/test/resolve-attributes.test.ts +++ b/packages/comark/test/resolve-attributes.test.ts @@ -101,6 +101,94 @@ describe('resolveAttributes (parseJson mode)', () => { }) }) +describe('HTML sink props', () => { + it('drops innerHTML / dangerouslySetInnerHTML / textContent from resolved attributes', () => { + const result = resolveAttributes( + { + innerHTML: '', + textContent: 'overlay', + dangerouslySetInnerHTML: { __html: '' }, + title: 'safe', + }, + makeRenderData() + ) + expect(result).toEqual({ title: 'safe' }) + }) + + it('drops sink props with any casing and with the :binding prefix', () => { + const result = resolveAttributes( + { + InnerHtml: '', + ':dangerouslySetInnerHTML': '{"__html":""}', + TEXTCONTENT: 'overlay', + id: 'keep', + }, + makeRenderData(), + { parseJson: true } + ) + expect(result).toEqual({ id: 'keep' }) + }) +}) + +describe('unsafe URL bindings (render-time hard floor)', () => { + it('drops :href bindings resolving to javascript: via dot-path', () => { + const result = resolveAttributes( + { ':href': 'frontmatter.home' }, + makeRenderData({ frontmatter: { home: 'javascript:alert(1)' } }) + ) + expect(result).toEqual({}) + }) + + it('drops :href bindings resolving to javascript: via JSON (parseJson mode)', () => { + const result = resolveAttributes({ ':href': '"javascript:alert(1)"' }, makeRenderData(), { parseJson: true }) + expect(result).toEqual({}) + }) + + it('drops :src bindings resolving to data:text/html', () => { + const result = resolveAttributes({ ':src': '"data:text/html,"' }, makeRenderData(), { + parseJson: true, + }) + expect(result).toEqual({}) + }) + + it('drops bindings whose resolved value is entity-encoded javascript:', () => { + const result = resolveAttributes( + { ':href': 'frontmatter.home' }, + makeRenderData({ frontmatter: { home: 'javascript:alert(1)' } }) + ) + expect(result).toEqual({}) + }) + + it('keeps safe URL bindings', () => { + const result = resolveAttributes( + { ':href': 'frontmatter.home' }, + makeRenderData({ frontmatter: { home: 'https://example.com' } }) + ) + expect(result).toEqual({ href: 'https://example.com' }) + }) + + it('keeps relative URL bindings', () => { + const result = resolveAttributes( + { ':href': 'frontmatter.home' }, + makeRenderData({ frontmatter: { home: '/about' } }) + ) + expect(result).toEqual({ href: '/about' }) + }) + + it('does not drop literal javascript: hrefs — parse-time validation is the plugin\u2019s job', () => { + const result = resolveAttributes({ href: 'javascript:alert(1)' }, makeRenderData()) + expect(result).toEqual({ href: 'javascript:alert(1)' }) + }) + + it('does not drop non-URL bindings with unsafe-looking strings', () => { + const result = resolveAttributes( + { ':title': 'frontmatter.t' }, + makeRenderData({ frontmatter: { t: 'javascript: is a scheme' } }) + ) + expect(result).toEqual({ title: 'javascript: is a scheme' }) + }) +}) + describe('parseMarkdown + resolveAttributes end-to-end (#364)', () => { const renderData = makeRenderData() diff --git a/test/bundle.test.ts b/test/bundle.test.ts index e2702db2..12dfbb51 100644 --- a/test/bundle.test.ts +++ b/test/bundle.test.ts @@ -67,7 +67,7 @@ describe('package bundle size', { timeout: 60_000 }, () => { "@comark/react": "43.6k (74 files)", "@comark/svelte": "43.9k (82 files)", "@comark/vue": "60.5k (78 files)", - "comark": "411k (154 files)", + "comark": "419k (154 files)", } `) })