fix: Changed the order for attribute checks slightly for safer hooks

cure53 · Jun 25, 2024 · fa542df · fa542df
1 parent b8b552c
commit fa542df
Show file tree

Hide file tree

Showing 9 changed files with 36 additions and 41 deletions.
diff --git a/dist/purify.cjs.js b/dist/purify.cjs.js
diff --git a/dist/purify.cjs.js.map b/dist/purify.cjs.js.map
diff --git a/dist/purify.es.mjs b/dist/purify.es.mjs
@@ -209,11 +209,9 @@ const DATA_ATTR = seal(/^data-[\-\w.\u00B7-\uFFFF]/); // eslint-disable-line no-
 const ARIA_ATTR = seal(/^aria-[\-\w]+$/); // eslint-disable-line no-useless-escape
 const IS_ALLOWED_URI = seal(/^(?:(?:(?:f|ht)tps?|mailto|tel|callto|sms|cid|xmpp):|[^a-z]|[a-z+.\-]+(?:[^a-z+.\-:]|$))/i // eslint-disable-line no-useless-escape
 );
-
 const IS_SCRIPT_OR_DATA = seal(/^(?:\w+script|data):/i);
 const ATTR_WHITESPACE = seal(/[\u0000-\u0020\u00A0\u1680\u180E\u2000-\u2029\u205F\u3000]/g // eslint-disable-line no-control-regex
 );
-
 const DOCTYPE_NAME = seal(/^html$/i);
 const CUSTOM_ELEMENT = seal(/^[a-z][.\w]*(-[.\w]+)+$/i);
 
@@ -248,7 +246,6 @@ const NODE_TYPE = {
   documentFragment: 11,
   notation: 12 // Deprecated
 };
-
 const getGlobal = function getGlobal() {
   return typeof window === 'undefined' ? null : window;
 };
@@ -1010,7 +1007,7 @@ function createDOMPurify() {
       return true;
     }
 
-    /* Remove any ocurrence of processing instructions */
+    /* Remove any occurrence of processing instructions */
     if (currentNode.nodeType === NODE_TYPE.progressingInstruction) {
       _forceRemove(currentNode);
       return true;
@@ -1179,6 +1176,13 @@ function createDOMPurify() {
       hookEvent.forceKeepAttr = undefined; // Allows developers to see this is a property they can set
       _executeHook('uponSanitizeAttribute', currentNode, hookEvent);
       value = hookEvent.attrValue;
+
+      /* Work around a security issue with comments inside attributes */
+      if (SAFE_FOR_XML && regExpTest(/((--!?|])>)|<\/(style|title)/i, value)) {
+        _removeAttribute(name, currentNode);
+        continue;
+      }
+
       /* Did the hooks approve of the attribute? */
       if (hookEvent.forceKeepAttr) {
         continue;
@@ -1198,12 +1202,6 @@ function createDOMPurify() {
         continue;
       }
 
-      /* Work around a security issue with comments inside attributes */
-      if (SAFE_FOR_XML && regExpTest(/((--!?|])>)|<\/(style|title)/i, value)) {
-        _removeAttribute(name, currentNode);
-        continue;
-      }
-
       /* Sanitize attribute content to be template-safe */
       if (SAFE_FOR_TEMPLATES) {
         arrayForEach([MUSTACHE_EXPR, ERB_EXPR, TMPLIT_EXPR], expr => {

diff --git a/dist/purify.es.mjs.map b/dist/purify.es.mjs.map
diff --git a/dist/purify.js b/dist/purify.js
diff --git a/dist/purify.js.map b/dist/purify.js.map
diff --git a/dist/purify.min.js b/dist/purify.min.js
diff --git a/dist/purify.min.js.map b/dist/purify.min.js.map
diff --git a/src/purify.js b/src/purify.js
@@ -1280,6 +1280,13 @@ function createDOMPurify(window = getGlobal()) {
       hookEvent.forceKeepAttr = undefined; // Allows developers to see this is a property they can set
       _executeHook('uponSanitizeAttribute', currentNode, hookEvent);
       value = hookEvent.attrValue;
+
+      /* Work around a security issue with comments inside attributes */
+      if (SAFE_FOR_XML && regExpTest(/((--!?|])>)|<\/(style|title)/i, value)) {
+        _removeAttribute(name, currentNode);
+        continue;
+      }
+
       /* Did the hooks approve of the attribute? */
       if (hookEvent.forceKeepAttr) {
         continue;
@@ -1299,12 +1306,6 @@ function createDOMPurify(window = getGlobal()) {
         continue;
       }
 
-      /* Work around a security issue with comments inside attributes */
-      if (SAFE_FOR_XML && regExpTest(/((--!?|])>)|<\/(style|title)/i, value)) {
-        _removeAttribute(name, currentNode);
-        continue;
-      }
-
       /* Sanitize attribute content to be template-safe */
       if (SAFE_FOR_TEMPLATES) {
         arrayForEach([MUSTACHE_EXPR, ERB_EXPR, TMPLIT_EXPR], (expr) => {