GHSA-r47g-fvhr-h676 DOMPurify: IN_PLACE mode preserves attributes of a clobbered root element, allowing XSS via attacker-controlled root DOM

• npm dompurify: < 3.4.6 → fixed in 3.4.6

IN_PLACE mode preserves attributes of a clobbered root element, allowing XSS via attacker-controlled root DOM CWE: CWE-79 (XSS — Improper Neutralization of Input During Web Page Generation) via CWE-693 (Protection Mechanism Failure — silent no-op when `_forceRemove` is called on a parent-less node) Summary When `DOMPurify.sanitize(root, { IN_PLACE: true })` is called and `root` is a `` whose own attributes carry an event handler (`onmouseover`, `onfocus`, `onclick`, etc.), a single descendant element with a `name=` attribute matching any of the property names `_isClobbered` checks (`nodeName`, `setAttribute`, `namespaceURI`, `insertBefore`, `hasChildNodes`, `childNodes`) is sufficient to bypass attribute sanitization on the root. `_forceRemove` silently no-ops because the root has no parent; the iterator drives on to `_sanitizeAttributes`, which early-returns on clobbered nodes — and the event handler attribute is never inspected. The sanitized return is the same root, with the handler live. This affects current `main` at `89da34e` (the just-landed DOM-clobbering hardening fix at `89da34e` addressed `_sanitizeAttachedShadowRoots` walk traversal, not the main `_sanitizeElements` / `_sanitizeAttributes` pipeline against the iterator-root node). Affected • DOMPurify ≤ 3.4.5, including `main` at `89da34e03ec17868e561f87f3747a9371b61a9e7` • Any caller that does `DOMPurify.sanitize(node, { IN_PLACE: true })` where `node` is built from untrusted HTML (e.g., parsed via `createElement('template').innerHTML = dirty` then `template.content.firstElementChild` handed in) Not affected: • String-input `DOMPurify.sanitize(dirtyString)` — the library builds the DOM itself inside `_initDocument`, the root is the cleanly-created document body, and clobber-named children of the body cannot shadow `body` named properties (HTMLBodyElement does not carry `[LegacyOverrideBuiltIns]`) • IN_PLACE where the root is not an HTMLFormElement • IN_PLACE where the attacker cannot place a clobber-named child inside the root Vulnerability details Code paths [A] — `_forceRemove` at `src/purify.ts:930-939`: ```ts const _forceRemove = function (node: Node): void { arrayPush(DOMPurify.removed, { element: node }); try { // eslint-disable-next-line unicorn/prefer-dom-node-remove getParentNode(node).removeChild(node); // [A1] throws when getParentNode returns null } catch (_) { remove(node); // [A2] WebIDL Node.remove() — spec-defined no-op } // when the node has no parent }; ``` When the iterator-root has no parent (the standard IN_PLACE case where the caller hands in a detached node), `getParentNode(node)` returns `null`, `null.removeChild(node)` throws, the catch falls to `remove(node)` — which per WebIDL is `Element.prototype.remove.call(node)`, and per spec does nothing if the node has no parent. Nothing about `_forceRemove`'s contract acknowledges this — the function appears to its callers as "the node is gone now," but the node is still in place. [B] — `_sanitizeAttributes` at `src/purify.ts:1490-1492`: ```ts const _sanitizeAttributes = function (currentNode: Element): void { _executeHooks(hooks.beforeSanitizeAttributes, currentNode, null); const { attributes } = currentNode; /* Check if we have attributes; if not we might have a text node */ if (!attributes || _isClobbered(currentNode)) { return; // [B] silently skips ALL attribute checks } // for clobbered nodes ... }; ``` The skip at `[B]` is deliberate — the intent is to avoid touching nodes the library has already decided to discard. The invariant the comment implies is *"if `_isClobbered`, then `_sanitizeElements` already removed this node, so we will never reach `_sanitizeAttributes` on it."* That invariant holds for every non-root node (their `_forceRemove` succeeds in detaching them), but fails for the iterator root in IN_PLACE mode. The mismatch is between [A] and [B]: [A] assumes "removal" means the node will not be observed again, and [B] assumes any clobbered node it sees has already been removed. Neither holds for the iterator root. A correct guard would either make `_forceRemove` fail loudly on parent-less nodes (so the caller can bail out of IN_PLACE entirely) or have `_sanitizeAttributes` strip attributes from clobbered roots before returning. Iterator call site `src/purify.ts:1850-1864` ignores the boolean return value of `_sanitizeElements`: ```ts const nodeIterator = _createNodeIterator(IN_PLACE ? dirty : body); while ((currentNode = nodeIterator.nextNode())) { _sanitizeElements(currentNode); // returns `true` if killed — IGNORED _sanitizeAttributes(currentNode); // runs unconditionally; relies on [B]'s skip ... } ``` If the return value were checked and `_sanitizeAttributes` skipped when the node was "killed," the bug would not exist as a discrete issue — but currently `_sanitizeAttributes` is the only line of defense for a node that `_sanitizeElements` could not actually detach. Why the clobber works In Chromium/WebKit/Firefox, `HTMLFormElement` carries the WebIDL `[LegacyOverrideBuiltIns]` extended attribute on its named-property getter. A descendant element with `name="X"` (or `id="X"`, for radio-button-like names) shadows the matching property on the form, including properties inherited from `Element`, `Node`, and `EventTarget` prototypes. This is the same primitive the just-landed `89da34e` fix addresses for shadow-root traversal, but `_isClobbered`'s typeof checks (and the bypass-by-detection-failure path here) are independent of that fix. Verified clobber targets (each name= value independently triggers `_isClobbered`): | `name=` value | property `_isClobbered` checks | typeof on clobbered form | |---|---|---| | `nodeName` | `typeof element.nodeName !== 'string'` | object (an ``) | | `setAttribute` | `typeof element.setAttribute !== 'function'` | object (not callable) — *but* ``/``/`` ARE callable; see "Note on callable elements" below | | `namespaceURI` | `typeof element.namespaceURI !== 'string'` | object | | `insertBefore` | `typeof element.insertBefore !== 'function'` | object | | `hasChildNodes` | `typeof element.hasChildNodes !== 'function'` | object | | `childNodes` | `!(element.childNodes && typeof element.childNodes.length === 'number')` | object — `` has no `.length` | | `attributes` | `!(element.attributes instanceof NamedNodeMap)` | object (an `` is not a NamedNodeMap) | | `textContent` | `typeof element.textContent !== 'string'` | object | | `removeChild` | `typeof element.removeChild !== 'function'` | object (non-callable) | | `removeAttribute` | `typeof element.removeAttribute !== 'function'` | object (non-callable) | Any single one of the ten property names in `_isClobbered`'s checklist is sufficient as the bypass trigger. Proof of concept (1) Minimal — runnable in a single browser context ```html const root = document.createElement('form'); root.setAttribute('onmouseover', 'window.__rooted = 1'); const clobber = document.createElement('input'); clobber.setAttribute('name', 'nodeName'); root.appendChild(clobber); // typeof root.nodeName === 'object' (an element), not 'string'. // _isClobbered fires; _forceRemove(root) becomes a no-op because root.parentNode === null. DOMPurify.sanitize(root, { IN_PLACE: true }); console.log('output:', root.outerHTML); // // ^^^^^^^^^^^^^^^^^^ event handler survived ^^^^^^^^^^^^^^^^^^ document.body.appendChild(root); root.dispatchEvent(new MouseEvent('mouseover', { bubbles: true })); console.log('handler fired:', window.__rooted === 1); // true </h…