pasteHTML should prevent script execution

[sijad]

if something like quill.pasteHTML('<img src="dfsdfds" onerror="alert(document)">') called, the alert(document) will be executed. I'm not sure if it's quill job to prevent this. I just want to make sure.

Steps for Reproduction

1. Visit http://codepen.io/sijad/pen/NRAAdj

Expected behavior:
Quill should sanitize unsafe attributes and tags.

Actual behavior:
See http://codepen.io/sijad/pen/NRAAdj

Platforms:

53.0.2785.116 (64-bit), osx 10.11.6

Version:

1.0.3

[jhchen]

Sanitization is better handled outside of Quill and I'd rather not impose a set of rules, which are error-prone anyways. There are legitimate use cases for a onerror handler on images for example though it is also a vector for attack. The right behavior is user dependent.

Instead Quill will follow React's example of providing a very clear warning in the method name.

[sijad]

thanks for your effort @jhchen
most script and cms store origin user input intact beside rendered (sanitized) version in database for when user need to edit previous inputs.

1. sanitizing user input before saving it in DB may produce some problem when user want to edit.
2. sanitizing in client side is also expensive (sanitize-html minified version is about ~180kb)

could be please give me some advice here?

both CKEditor and TinyMCE have filter content feature.

[sijad]

@jhchen is it possible to use DOMParser class instead of using innerHTML?

as delta do all other work (e.g. convert tags and attributes) i guess it'll really fix this issue. I didn't look enough in code so I may be wrong.

DOMParser is well supported in all new browser http://caniuse.com/#feat=xml-serializer

tmpdoc = new DOMParser().parseFromString('<img src="" onerror="alert(\'it will not execute!\')">', "text/html");
console.log(tmpdoc.body.innerHTML);

[jhchen]

HTML sanitization is a huge mess. I would recommend not using HTML at all as a form of input and Quill has Deltas for this reason. The bright engineers at Facebook working on React seems to feel the same.

DOMParser is a good suggestion though from @benbro's PR it does not appear a simple substitution works, though with some more modifications it might. Regardless it does not cover all vectors for attack when it comes to HTML input and it is outside Quill's responsibility.