Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions dist/purify.cjs.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/purify.cjs.js.map

Large diffs are not rendered by default.

27 changes: 22 additions & 5 deletions dist/purify.es.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1643,9 +1643,14 @@ function createDOMPurify() {
const _sanitizeElements = function _sanitizeElements(currentNode, root) {
/* Execute a hook if present */
_executeHooks(hooks.beforeSanitizeElements, currentNode, null);
/* A hook may have detached the node — treat it as removed (see the
detached-node comment after the uponSanitizeElement hook below). */
/* A hook may have detached the node - treat it as removed (see the
detached-node comment after the uponSanitizeElement hook below). On
the IN_PLACE path, neutralize the detached subtree first so a queued
resource handler on it cannot fire in page scope after we return. */
if (currentNode !== root && getParentNode(currentNode) === null) {
if (IN_PLACE) {
_neutralizeSubtree(currentNode);
}
return true;
}
/* Check if element is clobbered or can clobber */
Expand All @@ -1672,10 +1677,22 @@ function createDOMPurify() {
opposite of a node that is already safely gone. The walk root is
exempt: a detached IN_PLACE root is legitimate input and must still
be fully sanitized, and a kill-decision on it must keep hitting the
REPORT-3 throw. Nodes detached by hooks are the hook's
responsibility: they are not recorded in DOMPurify.removed and are
not neutralized by the post-walk IN_PLACE pass. */
REPORT-3 throw. Nodes detached by hooks stay the hook's
responsibility for placement: they are not recorded in
DOMPurify.removed, so the post-walk IN_PLACE pass (which iterates
DOMPurify.removed) does not reach them. But a hook-detached subtree
can still hold a queued resource-event handler - e.g. an <img onload>
that began loading when the caller built the live tree - which fires
in page scope after sanitize returns even though the handler never
reached the returned tree. That is the audit-5 F1 hazard, and the
documented node.remove() hook pattern walks straight into it. So on
the IN_PLACE path we neutralize the detached subtree inline here,
stripping its non-allow-listed attributes before returning, exactly
as the post-walk pass does for _forceRemove'd subtrees. */
if (currentNode !== root && getParentNode(currentNode) === null) {
if (IN_PLACE) {
_neutralizeSubtree(currentNode);
}
return true;
}
/* Remove mXSS vectors, processing instructions and risky comments */
Expand Down
2 changes: 1 addition & 1 deletion dist/purify.es.mjs.map

Large diffs are not rendered by default.

27 changes: 22 additions & 5 deletions dist/purify.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/purify.js.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/purify.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/purify.min.js.map

Large diffs are not rendered by default.

29 changes: 24 additions & 5 deletions src/purify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1862,9 +1862,15 @@ function createDOMPurify(window: WindowLike = getGlobal()): DOMPurify {
/* Execute a hook if present */
_executeHooks(hooks.beforeSanitizeElements, currentNode, null);

/* A hook may have detached the node — treat it as removed (see the
detached-node comment after the uponSanitizeElement hook below). */
/* A hook may have detached the node - treat it as removed (see the
detached-node comment after the uponSanitizeElement hook below). On
the IN_PLACE path, neutralize the detached subtree first so a queued
resource handler on it cannot fire in page scope after we return. */
if (currentNode !== root && getParentNode(currentNode) === null) {
if (IN_PLACE) {
_neutralizeSubtree(currentNode);
}

return true;
}

Expand Down Expand Up @@ -1897,10 +1903,23 @@ function createDOMPurify(window: WindowLike = getGlobal()): DOMPurify {
opposite of a node that is already safely gone. The walk root is
exempt: a detached IN_PLACE root is legitimate input and must still
be fully sanitized, and a kill-decision on it must keep hitting the
REPORT-3 throw. Nodes detached by hooks are the hook's
responsibility: they are not recorded in DOMPurify.removed and are
not neutralized by the post-walk IN_PLACE pass. */
REPORT-3 throw. Nodes detached by hooks stay the hook's
responsibility for placement: they are not recorded in
DOMPurify.removed, so the post-walk IN_PLACE pass (which iterates
DOMPurify.removed) does not reach them. But a hook-detached subtree
can still hold a queued resource-event handler - e.g. an <img onload>
that began loading when the caller built the live tree - which fires
in page scope after sanitize returns even though the handler never
reached the returned tree. That is the audit-5 F1 hazard, and the
documented node.remove() hook pattern walks straight into it. So on
the IN_PLACE path we neutralize the detached subtree inline here,
stripping its non-allow-listed attributes before returning, exactly
as the post-walk pass does for _forceRemove'd subtrees. */
if (currentNode !== root && getParentNode(currentNode) === null) {
if (IN_PLACE) {
_neutralizeSubtree(currentNode);
}

return true;
}

Expand Down
59 changes: 59 additions & 0 deletions test/test-suite.js
Original file line number Diff line number Diff line change
Expand Up @@ -1796,6 +1796,65 @@
}
);

// A hook that detaches a node via node.remove() (the documented removal
// pattern) takes the subtree out of the tree before the walker reaches
// its descendants, and hook-detached nodes are deliberately not recorded
// in DOMPurify.removed - so the post-walk IN_PLACE neutralize pass never
// sees them. Without an inline neutralize at the hook-detach early
// returns, a descendant that was already loading keeps its queued on*
// handler and fires in page scope after sanitize returns, even though the
// returned tree is clean. Same shape as the F1 tests above: onerror with
// no src, assert the attribute is gone. Covers both element hooks.
[
{ hook: 'uponSanitizeElement', label: 'uponSanitizeElement' },
{ hook: 'beforeSanitizeElements', label: 'beforeSanitizeElements' },
].forEach(({ hook, label }) => {
QUnit.test(
'IN_PLACE: ' +
label +
' node.remove() neutralizes the detached subtree (audit-5 F1)',
(assert) => {
const root = document.createElement('div');
root.innerHTML =
'<section><img id="tail" onerror="alert(1)"></section>' +
'<div>safe</div>';
const tail = root.querySelector('#tail');

DOMPurify.addHook(hook, (node) => {
if (node.nodeName === 'SECTION') {
node.remove();
}
});

try {
const ret = DOMPurify.sanitize(root, { IN_PLACE: true });

assert.equal(ret, root, 'returns the same in-place node');
assert.notOk(
ret.querySelector('section, #tail'),
'detached subtree is absent from the returned tree'
);
assert.strictEqual(
tail.getAttribute('onerror'),
null,
'on* handler stripped from the hook-detached descendant'
);

// App-side store-and-rerender must expose no executable sink.
const probe = document.createElement('div');
probe.innerHTML = ret.outerHTML + tail.outerHTML;
assert.notOk(
probe.querySelector('[onerror],[onload],script'),
'no executable sink survives serialize/reparse: ' +
probe.innerHTML
);
} finally {
DOMPurify.removeHook(hook);
}
}
);
});

// =======================================================================
// Config: FORBID_TAGS / FORBID_ATTR
// =======================================================================
Expand Down