Allow TinyMCE to convert safe embed/objects to audio/img/video #16834
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
trasher
reviewed
Mar 27, 2024
| @@ -3851,7 +3851,7 @@ public static function initEditorSystem($id, $rand = '', $display = true, $reado | |||
| $cache_suffix = '?v=' . FrontEnd::getVersionCacheKey(GLPI_VERSION); | |||
| $readonlyjs = $readonly ? 'true' : 'false'; | |||
|
|
|||
| $invalid_elements = 'applet,canvas,embed,form,object'; | |||
There was a problem hiding this comment.
Why remove embed and object? That honestly seems tags that should not be used.
There was a problem hiding this comment.
With convert_unsafe_embeds=true, they will be converted to audio, img or video tags. The purpose is to correctly handle pasting a content from an HTML page that uses object or embed for a media content.
Anyway, this is not mandatory, and cannot be done before tinymce/tinymce#9516 is fixed on TinyMCE side.
|
See tinymce/tinymce#9513, TinyMCE 6.8.x will never be considered as safe by npm, so this PR is not relevant anymore. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TinyMCE recently published 2 security advisories:
By default, we are not impacted by these security issues, but it is still preferable to upgrade to a version that is not marked as vulnerable. For the moment, the 6.8.1+ versions are still considered as vulnerable, but they should probably not, see tinymce/tinymce#9513.
For the vulnerability related to usage of
iframe, we already block them unless people set theGLPI_ALLOW_IFRAME_IN_RICH_TEXTtotrue. In this case, I think we should not set thesandbox_iframesoption to true (see https://www.tiny.cloud/docs/tinymce/6/security/#sandbox-iframes-option) as I guess people may sometimes use forms or scripts in their own iframes, and that is probably the reason they are enable them.For the vulnerability related to usage of
objectandembedelements, we do not allow them for the moment. I propose to allow them now, as they will be converted toaudio/img/videotags when possible, or intoiframetags otherwise thanks toconvert_unsafe_embeds. IfGLPI_ALLOW_IFRAME_IN_RICH_TEXTis not set totrue, the resultingiframetags will be discarded automatically (once tinymce/tinymce#9516 will be fixed).I keep this PR as a draft, waiting for tinymce/tinymce#9513 and tinymce/tinymce#9516 to be fixed.