Skip to content

Commit 9f0a07d

Browse files
authored
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
1 parent 137e3e0 commit 9f0a07d

10 files changed

Lines changed: 178 additions & 21 deletions

File tree

dist/purify.cjs.js

Lines changed: 25 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/purify.cjs.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/purify.es.mjs

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1566,9 +1566,15 @@ function createDOMPurify() {
15661566
/**
15671567
* Handle a node whose tag is forbidden or not allowlisted: keep
15681568
* allowed custom elements (false return exits _sanitizeElements
1569-
* early - namespace/fallback checks and the afterSanitizeElements
1570-
* hook are intentionally skipped for kept custom elements), else
1571-
* hoist content per KEEP_CONTENT and remove.
1569+
* early - the namespace and fallback-tag removal checks are
1570+
* intentionally skipped for kept custom elements), else hoist
1571+
* content per KEEP_CONTENT and remove.
1572+
*
1573+
* A kept custom element is the ONLY case in which this function
1574+
* returns false, so the caller uses that return value to run the
1575+
* afterSanitizeElements hook on the kept element and keep the
1576+
* element-hook lifecycle consistent with normal allowlisted
1577+
* elements (GHSA-c2j3-45gr-mqc4).
15721578
*
15731579
* @param currentNode the disallowed node
15741580
* @param tagName the node's transformCaseFunc'd tag name
@@ -1680,7 +1686,22 @@ function createDOMPurify() {
16801686
}
16811687
/* Remove element if anything forbids its presence */
16821688
if (FORBID_TAGS[tagName] || !(EXTRA_ELEMENT_HANDLING.tagCheck instanceof Function && EXTRA_ELEMENT_HANDLING.tagCheck(tagName)) && !ALLOWED_TAGS[tagName]) {
1683-
return _sanitizeDisallowedNode(currentNode, tagName);
1689+
const removed = _sanitizeDisallowedNode(currentNode, tagName);
1690+
/* A false return means the node is a custom element kept via
1691+
CUSTOM_ELEMENT_HANDLING - the only keep path through
1692+
_sanitizeDisallowedNode. Run afterSanitizeElements on it so the
1693+
element-hook lifecycle matches normal allowlisted elements: a
1694+
security policy applied in this hook (e.g. stripping an attribute
1695+
from every surviving element) must not silently skip kept custom
1696+
elements (GHSA-c2j3-45gr-mqc4). This mirrors the normal-element
1697+
tail below - the hook runs, then the walker's subsequent
1698+
_sanitizeAttributes pass sanitizes the element's attributes. The
1699+
deliberately skipped namespace and fallback-tag removal checks stay
1700+
skipped; they are removal decisions, not the hook contract. */
1701+
if (removed === false) {
1702+
_executeHooks(hooks.afterSanitizeElements, currentNode, null);
1703+
}
1704+
return removed;
16841705
}
16851706
/* Check whether element has a valid namespace.
16861707
Realm-safe check (GHSA-hpcv-96wg-7vj8): use the cached Node.prototype

dist/purify.es.mjs.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/purify.js

Lines changed: 25 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/purify.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/purify.min.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/purify.min.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/purify.ts

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1763,9 +1763,15 @@ function createDOMPurify(window: WindowLike = getGlobal()): DOMPurify {
17631763
/**
17641764
* Handle a node whose tag is forbidden or not allowlisted: keep
17651765
* allowed custom elements (false return exits _sanitizeElements
1766-
* early - namespace/fallback checks and the afterSanitizeElements
1767-
* hook are intentionally skipped for kept custom elements), else
1768-
* hoist content per KEEP_CONTENT and remove.
1766+
* early - the namespace and fallback-tag removal checks are
1767+
* intentionally skipped for kept custom elements), else hoist
1768+
* content per KEEP_CONTENT and remove.
1769+
*
1770+
* A kept custom element is the ONLY case in which this function
1771+
* returns false, so the caller uses that return value to run the
1772+
* afterSanitizeElements hook on the kept element and keep the
1773+
* element-hook lifecycle consistent with normal allowlisted
1774+
* elements (GHSA-c2j3-45gr-mqc4).
17691775
*
17701776
* @param currentNode the disallowed node
17711777
* @param tagName the node's transformCaseFunc'd tag name
@@ -1912,7 +1918,24 @@ function createDOMPurify(window: WindowLike = getGlobal()): DOMPurify {
19121918
) &&
19131919
!ALLOWED_TAGS[tagName])
19141920
) {
1915-
return _sanitizeDisallowedNode(currentNode, tagName);
1921+
const removed = _sanitizeDisallowedNode(currentNode, tagName);
1922+
1923+
/* A false return means the node is a custom element kept via
1924+
CUSTOM_ELEMENT_HANDLING - the only keep path through
1925+
_sanitizeDisallowedNode. Run afterSanitizeElements on it so the
1926+
element-hook lifecycle matches normal allowlisted elements: a
1927+
security policy applied in this hook (e.g. stripping an attribute
1928+
from every surviving element) must not silently skip kept custom
1929+
elements (GHSA-c2j3-45gr-mqc4). This mirrors the normal-element
1930+
tail below - the hook runs, then the walker's subsequent
1931+
_sanitizeAttributes pass sanitizes the element's attributes. The
1932+
deliberately skipped namespace and fallback-tag removal checks stay
1933+
skipped; they are removal decisions, not the hook contract. */
1934+
if (removed === false) {
1935+
_executeHooks(hooks.afterSanitizeElements, currentNode, null);
1936+
}
1937+
1938+
return removed;
19161939
}
19171940

19181941
/* Check whether element has a valid namespace.

test/test-suite.js

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2155,6 +2155,77 @@
21552155
});
21562156
});
21572157

2158+
// Regression: GHSA-c2j3-45gr-mqc4. A custom element kept via
2159+
// CUSTOM_ELEMENT_HANDLING must run through afterSanitizeElements just
2160+
// like a normal allowlisted element, so a security policy layered in
2161+
// that hook is not silently skipped for kept custom elements.
2162+
QUnit.test(
2163+
'afterSanitizeElements fires on kept custom elements',
2164+
(assert) => {
2165+
const seen = [];
2166+
DOMPurify.addHook('afterSanitizeElements', (node) => {
2167+
if (node.tagName) {
2168+
seen.push(node.tagName.toLowerCase());
2169+
}
2170+
});
2171+
DOMPurify.sanitize('<x-keep>a</x-keep><div>b</div>', {
2172+
CUSTOM_ELEMENT_HANDLING: { tagNameCheck: /^x-/ },
2173+
});
2174+
DOMPurify.removeAllHooks();
2175+
assert.ok(
2176+
seen.includes('x-keep'),
2177+
`hook must fire on kept custom element, saw: ${seen.join(',')}`
2178+
);
2179+
assert.ok(seen.includes('div'), 'hook still fires on normal element');
2180+
}
2181+
);
2182+
2183+
QUnit.test(
2184+
'afterSanitizeElements policy applies uniformly to custom elements',
2185+
(assert) => {
2186+
DOMPurify.addHook('afterSanitizeElements', (node) => {
2187+
if (node.hasAttribute && node.hasAttribute('data-bio')) {
2188+
node.removeAttribute('data-bio');
2189+
}
2190+
});
2191+
const clean = DOMPurify.sanitize(
2192+
'<div data-bio="x"></div><x-bio data-bio="x"></x-bio>',
2193+
{ CUSTOM_ELEMENT_HANDLING: { tagNameCheck: /^x-/ } }
2194+
);
2195+
DOMPurify.removeAllHooks();
2196+
assert.equal(clean, '<div></div><x-bio></x-bio>');
2197+
}
2198+
);
2199+
2200+
QUnit.test(
2201+
'afterSanitizeElements can remove a kept custom element',
2202+
(assert) => {
2203+
DOMPurify.addHook('afterSanitizeElements', (node) => {
2204+
if (node.tagName && node.tagName.toLowerCase() === 'x-evil') {
2205+
node.remove();
2206+
}
2207+
});
2208+
const clean = DOMPurify.sanitize('<x-evil>a</x-evil><x-ok>b</x-ok>', {
2209+
CUSTOM_ELEMENT_HANDLING: { tagNameCheck: /^x-/ },
2210+
});
2211+
DOMPurify.removeAllHooks();
2212+
assert.equal(clean, '<x-ok>b</x-ok>');
2213+
}
2214+
);
2215+
2216+
QUnit.test(
2217+
'kept custom element attributes are still sanitized (contract intact)',
2218+
(assert) => {
2219+
const clean = DOMPurify.sanitize(
2220+
'<x-a onclick="alert(1)" href="javascript:alert(1)"><img src=x onerror=alert(1)></x-a>',
2221+
{ CUSTOM_ELEMENT_HANDLING: { tagNameCheck: /^x-/ } }
2222+
);
2223+
assert.notOk(/onclick/i.test(clean), 'event handler stripped');
2224+
assert.notOk(/javascript:/i.test(clean), 'javascript: URI stripped');
2225+
assert.notOk(/onerror/i.test(clean), 'child payload sanitized');
2226+
}
2227+
);
2228+
21582229
// =======================================================================
21592230
// Config: ALLOW_ARIA_ATTR (#198)
21602231
// =======================================================================

0 commit comments

Comments
 (0)