[Critical][security] mXSS-detection fallback returns unescaped textContent → XSS via all HTML sinks #162

Severity: 🔴 Critical (verified exploit)

Location

src/security/sanitize-core.ts:356-361 — reached through every HTML sink: $el.html() / $$.html() (src/core/dom.ts, src/core/element.ts:236, src/core/collection.ts:229), .append()/.before()/.after(), and the default-sanitized bq-html directive (src/view/directives/html.ts:15).

Description

The anti-mutation-XSS double-parse ends with a fallback that returns raw, un-escaped textContent:

if (firstPass !== secondPass) {
  // Content mutated during re-parse - potential mXSS detected.
  return fragment.textContent ?? '';   // <-- RAW, UNESCAPED
}

Every caller assigns the return value of sanitizeHtmlCore to element.innerHTML / insertAdjacentHTML. Everywhere else the function returns serialized HTML (safe), but this branch returns plain text that is not HTML-escaped. When an attacker supplies the payload HTML-entity-encoded, the parser decodes it into a text node whose data is literally <img src=x onerror=alert(1)>. If the same input also contains a construct that makes serialize→re-parse unstable (trivial via HTML <table> foster-parenting), the "mXSS detected" branch fires and hands that raw markup back to the caller, which re-parses it as HTML and executes the payload. The defense meant to stop mXSS is itself the injection sink.

Reproduction (verified against the real sanitizeHtml + a real innerHTML sink under happy-dom)

const payload = '<a><table><a>&lt;img src=x onerror=alert(1)&gt;';
sanitizeHtmlCore(payload);               // => "<img src=x onerror=alert(1)>"
host.innerHTML = sanitizeHtml(payload);  // => live <img src="x" onerror="alert(1)"> fires

Observed DOM: <img src="x" onerror="alert(1)"> (img count = 1). The <a><table><a>… prefix makes the two serialization passes differ (nested-anchor + table foster-parenting), so the fallback returns fragment.textContent = the entity-decoded <img …> string. This is standard HTML behavior (identical across DOMParser and innerHTML in all standards browsers), not a happy-dom artifact. Reachable in the default, no-opt-out configuration.

Suggested fix

The fallback value is destined for an HTML sink, so it must be HTML-escaped (or empty):

if (firstPass !== secondPass) {
  return escapeHtml(fragment.textContent ?? '');
}

More broadly, reconsider the "return text on instability" design in favour of a re-sanitize-to-fixpoint loop (DOMPurify-style), which also avoids silent content destruction. Note the stripAllTags path at sanitize-core.ts:250 returns raw textContent too — audit any caller that treats a stripTags() result as HTML.

Filed as part of a full-codebase security & correctness audit.