-
-
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
ckeditor5-image fetchLocalImage() rejects Base64 due to Content Security Policy violation #7957
Comments
Hi all, just curious if anyone has any input on this issue? Any workarounds or assistance is greatly appreciated. |
Hi, I'm sorry for the late response, for some reason the issue slipped out our attention. In the mentioned Content Security Policy rules, |
Thanks for the reply. I was under the impression that using 'data' with connect-src was insecure. On https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/connect-src it says "This is insecure; an attacker can also inject arbitrary data: URIs. Use this sparingly and definitely not for scripts." Obviously in my example the CSP is local to the file so it would be less of a concern, but if I were to add 'data' to the CSP on our production server nginx config wouldn't that open us up to scripting attacks? |
First of all, thank you @giarcjc for such an amazing bug report ❤️ As for the issue itself, IDK if someone dig into that, but I'd be good to understand where in |
I've recently implemented a CSP and encountered the same issue. Would it be possible to not use Further |
Folks, any progress? The CSP solution is a no-go, given the security implications. |
There is one more scenario for this issue:
This will be very inconvenient for users who want to copy content from Word. In reality, I believe no one would copy/paste thing from Word one by one. Again, like other users above, adding |
TODO:
|
Fix (image): Allow pasting an image with data URL scheme in src, if strict CSP rules are defined. Closes #7957.
…rict CSP rules because the latter works one way only (once set, it cannot be reverted) and that affects all tests in other packages that will follow (see #7957).
After deploying our application to our server with CK editor, I noticed that if coping and pasting a selection of text that contains an image from MS Word, the image will fail to be embedded in the editor. This is not the case when copying only text or only an image from Word, but when the selection contains both text and an embedded image, a browser alert is triggered "TypeError: Failed to fetch", and the console log shows the error to be due to a Content Security Policy violation.
Initially I believed that this must be due to a misconfiguration of our server's CSP, because it does not happen when I use the CK demo at https://ckeditor.com/docs/ckeditor5/latest/features/image-upload/image-upload.html or at https://ckeditor.com/docs/ckeditor5/latest/features/pasting/paste-from-word.html, and I could not reproduce it locally, only on our servers.
However, I have tested this by changing the CSP in my server's nginx config to exactly match the recommended CSP configuration from this page: https://ckeditor.com/docs/ckeditor5/latest/builds/guides/integration/csp.html and I am still getting the error. It does not seem to be browser-related (tested in Chrome, FF and Safari).
After a lot of research and trial and error, I believe the problem is with the use of the native fetch api in fetchLocalImage() @ckeditor/ckeditor5-image/src/imageupload/utils.js when attempting to embed the placeholder image from the Base64 src. It seems to allow the promise rejection and image insertion as long as no upload adapter is configured (which might explain why it works in the demo) but with an upload adapter configured a blocking browser alert pops up and the image is not embedded.
By adding a few lines of code to the fetchLocalImage fn to effectively mimic the server's CSP, I have been able to reproduce this issue using a local build.
Simplified steps to reproduce (edited by Mgsy)
Original steps to reproduce:
Modified function now looks like this:
9a. In src/ckeditor.js, add
import SimpleUploadAdapter from '@ckeditor/ckeditor5-upload/src/adapters/simpleuploadadapter';
and add SimpleUploadAdapter to the Editor.builtinPlugins array.9b. Get a valid Bearer token for the server you upload to (I'm using the server/token from the Ck upload demo at https://ckeditor.com/docs/ckeditor5/latest/features/image-upload/image-upload.html), and assign to a 'token' variable.
9c. Modify sample/index.html by inserting the simple upload adapter config:
So the sample/index.html script should look like this (some code truncated for brevity)
FYI: When testing this against our servers with a custom-build upload adapter, I observed that even though the image is not embedded in the editor, the request to upload the image to our server succeeds (a link to the image location on our server is returned). Perhaps some error handling could be introduced to fall back to a stock placeholder SVG in the event that the Base64 image representation fails, so as not to block the eventual resolution of the final image url from the server?
✔️ Expected result
Both the image and the text from a Word doc can be copy/pasted and embedded in the editor without errors when using an Upload Adapter and the recommended Content Security Policy.
❌ Actual result
A browser alert warns of "TypeError: Failed to fetch" and the logs show the reason to be due to CSP violations on the attempt to fetch the Base64 image representation, and the image is not embedded into the editor (but the text still is).
📃 Other details
Word for Mac version: 16.40
Chrome v84.0.4147.125
FireFox: 79.0 (64-bit)
Safari: 13.1.2 (15609.3.5.1.3)
OS: macOS 10.15.5 and 10.15.6 (Catalina)
CKEditor version: 21.0.0
Installed CKEditor plugins: Just using the default plugins from https://ckeditor.com/ckeditor-5/online-builder/
Seems similar to these related issues:
#2493
#1710
#7915
If you'd like to see this fixed sooner, add a 👍 reaction to this post.
The text was updated successfully, but these errors were encountered: