fix(security): harden prop and URL validation against bypasses

docs/content/4.plugins/1.built-in/security.md

@@ -96,13 +96,22 @@ 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
 
 `href` and `src` values are decoded (URL-encoded and HTML entity variants included) and checked against a hard-coded block list. These protocols are **always** blocked, even if `allowedProtocols: ['*']` is set:
 
 `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

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)
     }
 

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="<img src=x onerror=alert(1)>"}\n::')
+    const html = await renderDocument(document)
+    expect(html).not.toContain('<img src=x onerror=alert(1)>')
+    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')
+  })
+})

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<string, string> = {
+  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&#58;alert(1)`
+ * through as a "relative" URL. Repeating the pass catches nested encodings
+ * like `&amp;#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