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
14.15k stars 735 forks source link

[bug] DomPurify hangs on indefinitely when using SAFE_FOR_TEMPLATES=true #985

Closed vicpara closed 3 months ago

vicpara commented 3 months ago

This issue proposes a bug which makes nodeJS spin CPU indefinitely when using 'SAFE_FOR_TEMPLATES: true'.

Background & Context


    const clean = DOMPurify.sanitize(html, {
      SAFE_FOR_TEMPLATES: true, // without this option it works fine
      SANITIZE_NAMED_PROPS: true,
      ALLOW_ARIA_ATTR: false,
      ALLOW_DATA_ATTR: false,

      KEEP_CONTENT: true,
      FORBID_TAGS: ['style', 'iframe', 'embed', 'track', 'object','frameset', 'video', 'svg', 'img', 'font', 'applet', 'frame', 'script', 'aside'],
      ALLOWED_ATTR: ['alt', 'src'],
      FORBID_ATTR: ['style'],
    })

The HTML that triggered the problem was saved into a Gist because was too big. Available here https://gist.github.com/vicpara/eb4ebeda28c774f9a5f0cd78e3aca961#file-gistfile1-txt

Bug

Input

Some HTML which is thrown at DOMPurify: https://gist.github.com/vicpara/eb4ebeda28c774f9a5f0cd78e3aca961#file-gistfile1-txt

Given output

none. it keeps the CPU busy and that's it.

Expected output

Cleaned HTML

cure53 commented 3 months ago

Heya, thanks for submitting this issue.

We had a look and wow, this is several megabytes of HTML :smile:

The SAFE_FOR_TEMPLATES option sadly needs to use rather slow and careful regexes, so, feeding those with several MB of HTML makes things a bit sluggish :slightly_smiling_face:

I wonder, is the option really needed in your case or can it be left away?

It usually is needed if the sanitized HTML is going into the parser of a template engine and possible expression interpolations then would execute JavaScript. If that is not the case, the option can be simply set to false or be omitted and all is fine.

vicpara commented 3 months ago

Agreed. I left it out and it all works well now.

cure53 commented 3 months ago

Oki doki