search

cure53 / DOMPurify

DOMPurify - a DOM-only, super-fast, uber-tolerant XSS sanitizer for HTML, MathML and SVG. DOMPurify works with a secure default, but offers a lot of configurability and hooks. Demo:
https://cure53.de/purify
Other
13.67k stars 698 forks source link

removing code if it includes '<' symbol #841

Closed sonikasharma1403 closed 1 year ago

sonikasharma1403 commented 1 year ago 

const content = `<!DOCTYPE html>
<html lang="en">
<head>
<title>RRRR</title>
<style>
.red {
  color: red
}
<!-- ENDS Mobile view media query to control image sizes for <599 px wide -->
</style>
</head>

<body>
</body>
</html>`;

try {
  let clean = DOMPurify.sanitize(content, {
    WHOLE_DOCUMENT: true,
  });
  console.log(clean);
} catch (e) {
  console.error('eee:::::', e);
}

notice that it removes the style tag..

now replace <!-- ENDS Mobile view media query to control image sizes for <599 px wide --> with <!-- ENDS Mobile view media query to control image sizes for 599 px wide -->.. notice that we have only removed the < less then symbol. and now dompurify returns the style tag

cure53 commented 1 year ago

That is expected behavior. Invalid HTML gets mangled by the browser/DOM before the sanitization kicks in. If you want to keep the style element no matter what, consider using the FORCE_BODY config option.

sonikasharma1403 commented 1 year ago

@cure53 -

  1. This is a valid HTML and works fine.
  2. i tried it with the config.. but it is still not keeping the style tag intact

tried the below html as well which has single line comment but the issue remains


<html lang="en">
<head>
<title>RRRR</title>
<style>
.red {
  color: red
}
.blue {
color: blue
}
/* ENDS Mobile view media query to control image sizes for <599 px wide */
</style>
</head>```

The concern here is that the `<` is added in a comment and hence should NOT be removed
cure53 commented 1 year ago

Ah, okay - I understand. We sadly have to do this because of an mXSS attack:

https://github.com/cure53/DOMPurify/blob/main/src/purify.js#L1003

Removing this behavior will cause mXSS bypasses.

sonikasharma1403 commented 1 year ago

@cure53

In the example above it's part of a comment. Correct me if I am wrong, but it won't cause an mXSS attack, right?

Can we have a flag to ignore the comments?

cure53 commented 1 year ago

It will - and no, sorry.