fastmail / Squire

The rich text editor for arbitrary HTML.
MIT License
4.77k stars 406 forks source link

Missing DOMPurify import #452

Closed jaboja closed 9 months ago

jaboja commented 10 months ago

The library tries to access a symbol DOMPurify despite never declaring or importing it. As a result it needs to be imported externally, and explicitly declared on the window element in order for Squire to run:

import DOMPurify from 'dompurify';
(window as any)["DOMPurify"] = DOMPurify;

If Squire is distributed as an NPM package, it would be better to explicitly declare dompurify as its dependency in package.json, and import it on its own, in its own scope, instead of expecting it to magically exist in the global scope.

Or maybe remove it from the code at all, and always require declaring the sanitize function explicitly, if importing it is expected to be explicit and conditional?

I'm not sure if there is a better solution, that would allow importing it conditionally, however expecting the symbol in global scope is a bad idea, as even if the lib is installed, poisoning the global scope is against the philosophy of NPM (especially as it needs to be done explicitly).

neilj commented 9 months ago

For those that use the library build directly rather than npm, this lets you just load DOMPurify and then Squire. If you don't import it and don't override the sanitize function it will crash, which is fine — better than working with a big security hole. I don't plan to change this.