-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[5.1] Allow PDF embeding again #43716
[5.1] Allow PDF embeding again #43716
Conversation
I have tested this item ✅ successfully on 46a404f This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43716. |
It's not broken and it's not a "bug". It's exactly doing what it's supposed to do: it limits the possible actions that an iframe can perform to protect the user from unexpected script execution. So, the question actually is: does the CMS want to enforce the strictest possible security configuration for the editor, at least by default? Or is the comfort in the content creation process more important? That's a question that's probably worth a vote in the production department meeting. |
I'm for security, of course, but it should at least be possible to embed a local PDF ... |
I have tested this item ✅ successfully on 46a404f This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43716. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43716. |
TBH, I have no idea why it is called security, We just making life for average User more complicated. They can just switch to codemirror, or none editor, and all this "theorethical security" will disapears. |
We are not talking about a CORS but about user experience and perception. Imagine an editor copy&pasting an iframe tag to an external site into the editor window of his companies intranet. On paste, the iframe will be loaded automatically - and without sandboxing, that iframe could i.e. trigger a file download. The user is now under the impression that this download is triggered by a trusted source, which is the Joomla administrator interface of his intranet - whereas the download is actually triggered by the untrusted iframe site. I'm not necessarily saying, that this is an important threat scenario for the average user, I just want to point out that the issue is not related to crossorigin or samesite policies. |
We can make a TinyMCE plugin, that shows an alert for potentialy dangerous iframe (and let User decide what to do), when User insert something in the editor. instead of totaly blocking everything. |
Maybe its an option to extend the TinyMCE plugin with the basic configuration for the sandbox parameters so a domain could be added to allow the PDF iframe? |
Now with parameter. Ready to test again :) |
I have tested this item ✅ successfully on 12ac335 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43716. |
1 similar comment
I have tested this item ✅ successfully on 12ac335 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43716. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43716. |
Co-authored-by: Brian Teeman <brian@teeman.net>
Thank you @bembelimen and also for testing and review @drmenzelit @SniperSister @RickR2H @Fedik @brianteeman |
joomla/joomla-cms#43654 + joomla/joomla-cms#43716 + joomla/joomla-cms#43723 - (только для др. пакета) joomla/joomla-cms#43717 + joomla/joomla-cms#43754 - (только для др. пакетов) joomla/joomla-cms#43765 - (только для др. пакетов)
joomla/joomla-cms#43654 + joomla/joomla-cms#43716 + joomla/joomla-cms#43723 - (тільки для ін. пакету) joomla/joomla-cms#43717 + joomla/joomla-cms#43754 - (тільки для ін. пакетів) joomla/joomla-cms#43765 - (тільки для ін. пакетів)
Pull Request for Issue #43407 & #43365 .
Summary of Changes
The security hardening adds an empty "sandbox" attribute to every iframe (read more). This prevents PDFs insert by the media manager loaded (it needs more fine granulated sandbox values)
With TinyMCE 7 you can whitelist certain domains, which will overcome this problem, but till then, this filter is super strict and not usable.
Testing Instructions
Use the media manager to embed a PDF. Save the article and open article in frontpage.
Actual result BEFORE applying this Pull Request
Expected result AFTER applying this Pull Request
Go to the TinyMCE configuration and disable the following option:
PDF is visible