fix: addressed an issue with hook-handling for CE sanitization (#1527)

* fix: addressed an issue with hook-handling for CE sanitization, thanks @Rikuxx0

* chore: rebuild dist to sync purify.min.js + sourcemaps after main merge
This commit is contained in:
Cure53
2026-07-11 13:23:47 +02:00
committed by GitHub
parent dc37bb1d02
commit 044b58b401
10 changed files with 178 additions and 21 deletions
+25 -4
View File
@@ -1568,9 +1568,15 @@ function createDOMPurify() {
/**
* Handle a node whose tag is forbidden or not allowlisted: keep
* allowed custom elements (false return exits _sanitizeElements
* early - namespace/fallback checks and the afterSanitizeElements
* hook are intentionally skipped for kept custom elements), else
* hoist content per KEEP_CONTENT and remove.
* early - the namespace and fallback-tag removal checks are
* intentionally skipped for kept custom elements), else hoist
* content per KEEP_CONTENT and remove.
*
* A kept custom element is the ONLY case in which this function
* returns false, so the caller uses that return value to run the
* afterSanitizeElements hook on the kept element and keep the
* element-hook lifecycle consistent with normal allowlisted
* elements (GHSA-c2j3-45gr-mqc4).
*
* @param currentNode the disallowed node
* @param tagName the node's transformCaseFunc'd tag name
@@ -1682,7 +1688,22 @@ function createDOMPurify() {
}
/* Remove element if anything forbids its presence */
if (FORBID_TAGS[tagName] || !(EXTRA_ELEMENT_HANDLING.tagCheck instanceof Function && EXTRA_ELEMENT_HANDLING.tagCheck(tagName)) && !ALLOWED_TAGS[tagName]) {
return _sanitizeDisallowedNode(currentNode, tagName);
const removed = _sanitizeDisallowedNode(currentNode, tagName);
/* A false return means the node is a custom element kept via
CUSTOM_ELEMENT_HANDLING - the only keep path through
_sanitizeDisallowedNode. Run afterSanitizeElements on it so the
element-hook lifecycle matches normal allowlisted elements: a
security policy applied in this hook (e.g. stripping an attribute
from every surviving element) must not silently skip kept custom
elements (GHSA-c2j3-45gr-mqc4). This mirrors the normal-element
tail below - the hook runs, then the walker's subsequent
_sanitizeAttributes pass sanitizes the element's attributes. The
deliberately skipped namespace and fallback-tag removal checks stay
skipped; they are removal decisions, not the hook contract. */
if (removed === false) {
_executeHooks(hooks.afterSanitizeElements, currentNode, null);
}
return removed;
}
/* Check whether element has a valid namespace.
Realm-safe check (GHSA-hpcv-96wg-7vj8): use the cached Node.prototype
+1 -1
View File
File diff suppressed because one or more lines are too long
+25 -4
View File
@@ -1566,9 +1566,15 @@ function createDOMPurify() {
/**
* Handle a node whose tag is forbidden or not allowlisted: keep
* allowed custom elements (false return exits _sanitizeElements
* early - namespace/fallback checks and the afterSanitizeElements
* hook are intentionally skipped for kept custom elements), else
* hoist content per KEEP_CONTENT and remove.
* early - the namespace and fallback-tag removal checks are
* intentionally skipped for kept custom elements), else hoist
* content per KEEP_CONTENT and remove.
*
* A kept custom element is the ONLY case in which this function
* returns false, so the caller uses that return value to run the
* afterSanitizeElements hook on the kept element and keep the
* element-hook lifecycle consistent with normal allowlisted
* elements (GHSA-c2j3-45gr-mqc4).
*
* @param currentNode the disallowed node
* @param tagName the node's transformCaseFunc'd tag name
@@ -1680,7 +1686,22 @@ function createDOMPurify() {
}
/* Remove element if anything forbids its presence */
if (FORBID_TAGS[tagName] || !(EXTRA_ELEMENT_HANDLING.tagCheck instanceof Function && EXTRA_ELEMENT_HANDLING.tagCheck(tagName)) && !ALLOWED_TAGS[tagName]) {
return _sanitizeDisallowedNode(currentNode, tagName);
const removed = _sanitizeDisallowedNode(currentNode, tagName);
/* A false return means the node is a custom element kept via
CUSTOM_ELEMENT_HANDLING - the only keep path through
_sanitizeDisallowedNode. Run afterSanitizeElements on it so the
element-hook lifecycle matches normal allowlisted elements: a
security policy applied in this hook (e.g. stripping an attribute
from every surviving element) must not silently skip kept custom
elements (GHSA-c2j3-45gr-mqc4). This mirrors the normal-element
tail below - the hook runs, then the walker's subsequent
_sanitizeAttributes pass sanitizes the element's attributes. The
deliberately skipped namespace and fallback-tag removal checks stay
skipped; they are removal decisions, not the hook contract. */
if (removed === false) {
_executeHooks(hooks.afterSanitizeElements, currentNode, null);
}
return removed;
}
/* Check whether element has a valid namespace.
Realm-safe check (GHSA-hpcv-96wg-7vj8): use the cached Node.prototype
+1 -1
View File
File diff suppressed because one or more lines are too long
+25 -4
View File
@@ -1572,9 +1572,15 @@
/**
* Handle a node whose tag is forbidden or not allowlisted: keep
* allowed custom elements (false return exits _sanitizeElements
* early - namespace/fallback checks and the afterSanitizeElements
* hook are intentionally skipped for kept custom elements), else
* hoist content per KEEP_CONTENT and remove.
* early - the namespace and fallback-tag removal checks are
* intentionally skipped for kept custom elements), else hoist
* content per KEEP_CONTENT and remove.
*
* A kept custom element is the ONLY case in which this function
* returns false, so the caller uses that return value to run the
* afterSanitizeElements hook on the kept element and keep the
* element-hook lifecycle consistent with normal allowlisted
* elements (GHSA-c2j3-45gr-mqc4).
*
* @param currentNode the disallowed node
* @param tagName the node's transformCaseFunc'd tag name
@@ -1686,7 +1692,22 @@
}
/* Remove element if anything forbids its presence */
if (FORBID_TAGS[tagName] || !(EXTRA_ELEMENT_HANDLING.tagCheck instanceof Function && EXTRA_ELEMENT_HANDLING.tagCheck(tagName)) && !ALLOWED_TAGS[tagName]) {
return _sanitizeDisallowedNode(currentNode, tagName);
const removed = _sanitizeDisallowedNode(currentNode, tagName);
/* A false return means the node is a custom element kept via
CUSTOM_ELEMENT_HANDLING - the only keep path through
_sanitizeDisallowedNode. Run afterSanitizeElements on it so the
element-hook lifecycle matches normal allowlisted elements: a
security policy applied in this hook (e.g. stripping an attribute
from every surviving element) must not silently skip kept custom
elements (GHSA-c2j3-45gr-mqc4). This mirrors the normal-element
tail below - the hook runs, then the walker's subsequent
_sanitizeAttributes pass sanitizes the element's attributes. The
deliberately skipped namespace and fallback-tag removal checks stay
skipped; they are removal decisions, not the hook contract. */
if (removed === false) {
_executeHooks(hooks.afterSanitizeElements, currentNode, null);
}
return removed;
}
/* Check whether element has a valid namespace.
Realm-safe check (GHSA-hpcv-96wg-7vj8): use the cached Node.prototype
+1 -1
View File
File diff suppressed because one or more lines are too long
+1 -1
View File
File diff suppressed because one or more lines are too long
+1 -1
View File
File diff suppressed because one or more lines are too long
+27 -4
View File
@@ -1763,9 +1763,15 @@ function createDOMPurify(window: WindowLike = getGlobal()): DOMPurify {
/**
* Handle a node whose tag is forbidden or not allowlisted: keep
* allowed custom elements (false return exits _sanitizeElements
* early - namespace/fallback checks and the afterSanitizeElements
* hook are intentionally skipped for kept custom elements), else
* hoist content per KEEP_CONTENT and remove.
* early - the namespace and fallback-tag removal checks are
* intentionally skipped for kept custom elements), else hoist
* content per KEEP_CONTENT and remove.
*
* A kept custom element is the ONLY case in which this function
* returns false, so the caller uses that return value to run the
* afterSanitizeElements hook on the kept element and keep the
* element-hook lifecycle consistent with normal allowlisted
* elements (GHSA-c2j3-45gr-mqc4).
*
* @param currentNode the disallowed node
* @param tagName the node's transformCaseFunc'd tag name
@@ -1912,7 +1918,24 @@ function createDOMPurify(window: WindowLike = getGlobal()): DOMPurify {
) &&
!ALLOWED_TAGS[tagName])
) {
return _sanitizeDisallowedNode(currentNode, tagName);
const removed = _sanitizeDisallowedNode(currentNode, tagName);
/* A false return means the node is a custom element kept via
CUSTOM_ELEMENT_HANDLING - the only keep path through
_sanitizeDisallowedNode. Run afterSanitizeElements on it so the
element-hook lifecycle matches normal allowlisted elements: a
security policy applied in this hook (e.g. stripping an attribute
from every surviving element) must not silently skip kept custom
elements (GHSA-c2j3-45gr-mqc4). This mirrors the normal-element
tail below - the hook runs, then the walker's subsequent
_sanitizeAttributes pass sanitizes the element's attributes. The
deliberately skipped namespace and fallback-tag removal checks stay
skipped; they are removal decisions, not the hook contract. */
if (removed === false) {
_executeHooks(hooks.afterSanitizeElements, currentNode, null);
}
return removed;
}
/* Check whether element has a valid namespace.
+71
View File
@@ -2155,6 +2155,77 @@
});
});
// Regression: GHSA-c2j3-45gr-mqc4. A custom element kept via
// CUSTOM_ELEMENT_HANDLING must run through afterSanitizeElements just
// like a normal allowlisted element, so a security policy layered in
// that hook is not silently skipped for kept custom elements.
QUnit.test(
'afterSanitizeElements fires on kept custom elements',
(assert) => {
const seen = [];
DOMPurify.addHook('afterSanitizeElements', (node) => {
if (node.tagName) {
seen.push(node.tagName.toLowerCase());
}
});
DOMPurify.sanitize('<x-keep>a</x-keep><div>b</div>', {
CUSTOM_ELEMENT_HANDLING: { tagNameCheck: /^x-/ },
});
DOMPurify.removeAllHooks();
assert.ok(
seen.includes('x-keep'),
`hook must fire on kept custom element, saw: ${seen.join(',')}`
);
assert.ok(seen.includes('div'), 'hook still fires on normal element');
}
);
QUnit.test(
'afterSanitizeElements policy applies uniformly to custom elements',
(assert) => {
DOMPurify.addHook('afterSanitizeElements', (node) => {
if (node.hasAttribute && node.hasAttribute('data-bio')) {
node.removeAttribute('data-bio');
}
});
const clean = DOMPurify.sanitize(
'<div data-bio="x"></div><x-bio data-bio="x"></x-bio>',
{ CUSTOM_ELEMENT_HANDLING: { tagNameCheck: /^x-/ } }
);
DOMPurify.removeAllHooks();
assert.equal(clean, '<div></div><x-bio></x-bio>');
}
);
QUnit.test(
'afterSanitizeElements can remove a kept custom element',
(assert) => {
DOMPurify.addHook('afterSanitizeElements', (node) => {
if (node.tagName && node.tagName.toLowerCase() === 'x-evil') {
node.remove();
}
});
const clean = DOMPurify.sanitize('<x-evil>a</x-evil><x-ok>b</x-ok>', {
CUSTOM_ELEMENT_HANDLING: { tagNameCheck: /^x-/ },
});
DOMPurify.removeAllHooks();
assert.equal(clean, '<x-ok>b</x-ok>');
}
);
QUnit.test(
'kept custom element attributes are still sanitized (contract intact)',
(assert) => {
const clean = DOMPurify.sanitize(
'<x-a onclick="alert(1)" href="javascript:alert(1)"><img src=x onerror=alert(1)></x-a>',
{ CUSTOM_ELEMENT_HANDLING: { tagNameCheck: /^x-/ } }
);
assert.notOk(/onclick/i.test(clean), 'event handler stripped');
assert.notOk(/javascript:/i.test(clean), 'javascript: URI stripped');
assert.notOk(/onerror/i.test(clean), 'child payload sanitized');
}
);
// =======================================================================
// Config: ALLOW_ARIA_ATTR (#198)
// =======================================================================