-
Notifications
You must be signed in to change notification settings - Fork 338
🔒️Unsafe attachments #663
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
Merged
Merged
🔒️Unsafe attachments #663
Conversation
This file contains hidden or 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
f6530be
to
2c9132b
Compare
lunika
reviewed
Feb 28, 2025
2c9132b
to
b6696fa
Compare
lunika
approved these changes
Mar 3, 2025
b6696fa
to
a5a47cb
Compare
The frontend cannot access custom headers of a file, so we need to add a flag in the filename. We add the `unsafe` flag in the filename to indicate that the file is unsafe. Previous filename: "/{UUID4}.{extension}" New filename: "/{UUID4}-unsafe.{extension}"
Since the new design implementation, the alert error was not looking good. This commit improves the design of the alert error.
It can happen that the user is authenticated then the token is expired. The authenticated state should be updated to false in this case.
We want to prevent the user to open unsafe images in the browser. We blocked the click on the images. To download them, the user will have to use the download button.
Blocknote download button opens the file in a new tab, which could be not secure because of XSS attacks. We replace the download button with a new one that downloads the file instead of opening it in a new tab. Some files are flags as unsafe (SVG / js / exe), for these files we add a confirmation modal before downloading the file to prevent the user from downloading a file that could be harmful. In the future, we could add other security layers from this model, to analyze the file before downloading it by example.
We moved the toolbar components in BlockNoteToolBar folder.
On the media upload endpoint, we want to set the content-disposition header. Its value is based on the uploaded file mime-type and if flagged as unsafe. If the file is not an image or is unsafe then the contentDisposition is set to attachment to force its download. Otherwise, we set it to inline.
The media route is managed by nginx. On this route we want to add the Content-Security-Header to forbid fetching any resources. See : https://content-security-policy.com/
We added content-security-policy on nginx. It should be safe to allow svg files now. We remove the svg file from the unsafe attachments list. We adapt the tests accordingly.
a5a47cb
to
62d5c8d
Compare
Merged
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.
Purpose
We improve the security of unsafe files.
Proposal
We decided to sanitize markup files.
We will prevent users to open files from a new tab, the files will be directly downloaded.
We alert the users when they are about to download unsafe files with a confirmation modal.
We could implement other security layer in future development inside this confirmation modal, like proposing jecliqueoupas.
Demo