- Issue created by @plopesc
- 🇫🇷France mably
Hi @plopesc, how are we supposed to sanitize it exactly?
It looks like it could be anything in there, here is the code adding the attribute in the LazyLoad field formatter for example:
$element[$delta] = [ '#type' => 'container', '#attributes' => [ 'data-video-embed-field-lazy' => (string) $this->renderer->render($videos[$delta]), 'class' => ['video-embed-field-lazy'],
- 🇫🇷France prudloff Lille
DOMPurify (https://github.com/cure53/DOMPurify) could be used for this, but it would add a new dependency.
- 🇫🇷France prudloff Lille
It's true that this can be mitigated by not allowing the data-video-embed-field-modal in CKE config.
However, there are valid reasons to allow data-* attributes and they are not dangerous by themselves.video_embed_field turns a safe attribute into dangerous markup so it should be this module's responsibility to ensure the markup is safe.
- 🇫🇷France mably
Looks like it would be a huge architectural change then, as the embed rendered code could have some valid javascript code in it.
What would be the best way to do that then? Load the rendered code using Ajax?
- 🇫🇷France prudloff Lille
I see several ways to fix this:
- Sanitize the value of data-video-embed-field-modal with DOMPurify before using it. But if the embed can contain legitimate JS this would remove it.
- Insert the embed somewhere in the page (in a div tag with
display:none;
for example) and only have a selector in the data attribute. - Render the embed in a drupalSettings variable and use this variable to generate the dialog.
- Load the embed with AJAX.
- 🇫🇷France mably
Thanks for the suggestions.
So one of these solutions will have to be implemented before we merge back the CKEditor plugins into the 3.x dev branch.