fixup! fix: escape unsafe patterns in elements, comment nodes and pro…

…cessing instructions
angular · Oct 1, 2023 · aa16603 · aa16603
1 parent e4e1c95
commit aa16603
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 17 deletions.
diff --git a/lib/NodeUtils.js b/lib/NodeUtils.js
@@ -164,23 +164,7 @@ function escapeClosingCommentTag(rawContent) {
   if (!CLOSING_COMMENT_REGEXP.test(rawContent)) {
     return rawContent; // fast path
   }
-
-  const matches = rawContent.matchAll(CLOSING_COMMENT_REGEXP_GLOBAL);
-
-  let result = '';
-  let lastIndex = 0;
-  for (const match of matches) {
-    const lastMatchingCharIndex = match.index + match[0].length;
-    result += rawContent.substring(lastIndex, lastMatchingCharIndex) + '&gt;';
-    // Skip over the `>` char, since we've replaced it above.
-    // Start the next iteration with a char following the `>` one.
-    lastIndex = lastMatchingCharIndex + 1;
-  }
-  // Add remaining contents of a string after the last `>` char.
-  if (lastIndex < rawContent.length) {
-    result += rawContent.substring(lastIndex);
-  }
-  return result;
+  return rawContent.replaceAll('-->', '--&gt;').replaceAll('--!>', '--!&gt;');
 }
 
 /**

diff --git a/test/xss.js b/test/xss.js
@@ -212,6 +212,10 @@ exports.oneRawTextTagInsideAnotherOne = function() {
   style.appendChild(xmp);
   document.body.appendChild(style);
 
+  document.body.serialize().should.equal(
+    '<style><xmp>&lt;/style><script>alert(1)</script></xmp></style>'
+  );
+
   const html = document.serialize();
   return alertFired(html).should.eventually.be.false('alert fired for: ' + html);
 }
@@ -224,6 +228,10 @@ exports.xssInAttributeInsideRawTextTag = function() {
   xmp.appendChild(div);
   document.body.appendChild(xmp);
 
+  document.body.serialize().should.equal(
+    '<xmp><div title="&lt;/xmp&gt;&lt;script&gt;alert(1)&lt;/script&gt;"></div></xmp>'
+  );
+
   const html = document.serialize();
   return alertFired(html).should.eventually.be.false('alert fired for: ' + html);
 }
@@ -235,6 +243,10 @@ exports.commentNodeInsideRawTextTag = function() {
   xmp.appendChild(comment);
   document.body.appendChild(xmp);
 
+  document.body.serialize().should.equal(
+    '<xmp><!--&lt;/xmp><script>alert(1)</script>--></xmp>'
+  );
+
   const html = document.serialize();
   return alertFired(html).should.eventually.be.false('alert fired for: ' + html);
 }
@@ -245,6 +257,10 @@ exports.alternativeEndTagForRawTextTag = function() {
   style.textContent = "</style  /foobar><script>alert(1)</script>";
   document.body.appendChild(style);
 
+  document.body.serialize().should.equal(
+    '<style>&lt;/style  /foobar><script>alert(1)</script></style>'
+  );
+
   const html = document.serialize();
   return alertFired(html).should.eventually.be.false('alert fired for: ' + html);
 }
@@ -254,6 +270,10 @@ exports.badCommentNode = function() {
   const comment = document.createComment('--><script>alert(1)</script>');
   document.body.appendChild(comment);
 
+  document.body.serialize().should.equal(
+    '<!----&gt;<script>alert(1)</script>-->'
+  );
+
   const html = document.serialize();
   return alertFired(html).should.eventually.be.false('alert fired for: ' + html);
 }
@@ -263,6 +283,10 @@ exports.anotherBadCommentNode = function() {
   const comment = document.createComment('--!><script>alert(1)</script>');
   document.body.appendChild(comment);
 
+  document.body.serialize().should.equal(
+    '<!----!&gt;<script>alert(1)</script>-->'
+  );
+
   const html = document.serialize();
   return alertFired(html).should.eventually.be.false('alert fired for: ' + html);
 }
@@ -272,6 +296,10 @@ exports.badProcessingInstruction = function() {
   const pi = document.createProcessingInstruction("bad", "><script>alert(1)</script>");
   document.body.appendChild(pi);
 
+  document.body.serialize().should.equal(
+    '<?bad &gt;<script&gt;alert(1)</script&gt;?>'
+  );
+
   const html = document.serialize();
   return alertFired(html).should.eventually.be.false('alert fired for: ' + html);
 }