leizongmin / js-xss

Sanitize untrusted HTML (to prevent XSS) with a configuration specified by a Whitelist
http://jsxss.com
Other
5.21k stars 630 forks source link

[Security] Fix ReDoS #239

Closed ready-research closed 2 years ago

ready-research commented 3 years ago

Fix ReDoS

Reported in https://www.huntr.dev/bounties/8bdc9cfb-4328-4655-a480-0b2403f16f52/ you can access this using GitHub. Please validate using Mark as valid and also confirm the fix. Thank you.

leizongmin commented 3 years ago

It seems this patch is not works correctly. Here is am example:

// current version
function stripCommentTagCurrent(html) {
  return html.replace(/<!--[\s\S]*?-->/g, "");
}

// patch version
function stripCommentTagNew(html) {
  return html.replace(/<!--(?:(?!<!--[\s\S])*?)-->/g, "");
}

const list = [
  "<!-- hello -->",
  "<!--hello-->",
  "<!-- <!-- <!-- hello --> --> -->",
];
for (var i = 0; i < list.length; i++) {
  var s = list[i];
  console.log("input:         ", s);
  console.log("current output:", stripCommentTagCurrent(s));
  console.log("new output:    ", stripCommentTagNew(s));
  console.log("");
}

Outpus:

input:          <!-- hello -->
current output:
new output:     <!-- hello -->

input:          <!--hello-->
current output:
new output:     <!--hello-->

input:          <!-- <!-- <!-- hello --> --> -->
current output:  --> -->
new output:     <!-- <!-- <!-- hello --> --> -->
leizongmin commented 3 years ago

Still have problems with this case:

input:          <!-- <!-- <!-- hello --> --> -->
current output:  --> -->
new output:     <!-- <!--  --> -->
ready-research commented 3 years ago

@leizongmin Check this one once