Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Improve handling of svg files #7032

Closed
wants to merge 17 commits into from

Conversation

alexanderstephan
Copy link
Contributor

@alexanderstephan alexanderstephan commented Oct 25, 2021

Description

This PR adds a sanitizer that enables use of dataURLs for svg Blobs. Right now all files with the MIME-type image/svg+xml are getting converted to a application/octet-stream, which breaks the lightbox view.

How it currently looks like:

image

How it should look like:

image

A proper lightbox is displayed.

Security

It also fixes a security issue where SVGs can be forwarded to e2e encrypted rooms and possibly steal the encryption keys.

I would highly appreciate feedback regarding the use of DOMPurify. Especially the way I make tags work with the sanitizer. I did this because during my testing many SVG made heavy use of them.

Related issues

The idea of using DOMPurify was originally mentioned here as I found out later: element-hq/element-web#2581

I am looking into looking into this specific issue next, but for now it should fix the following issues:

Current status:

  • I added DOMPurify as a dependency
  • I implemented and tested sanitizing svgs and opening them in a lightbox
  • I tested that the thumbnails also work as expected in e2e encrypted rooms
  • I expanded the comments regarding MIME-type filtering and my implementation
  • Affirm that a thumbnail is always generated when it should
  • Review use of DOMPurify
  • Fix scaling
  • Prevent preview of large SVGs
  • Write tests

Signed-off-by: Alexander Stephan alexander.stephan@tum.de


Here's what your changelog entry will look like:

✨ Features

Preview: https://61771aa1667ae2e7582564a3--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

@t3chguy t3chguy added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Oct 25, 2021
@MadLittleMods MadLittleMods added the Z-Community-PR Issue is solved by a community member's PR label Jun 1, 2022
@alexanderstephan
Copy link
Contributor Author

Sadly, not having the time to finish this. Although I might come back to this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants