Skip to content
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

Switch back to using blob URLs for rendering e2e attachments #1864

Merged
merged 3 commits into from
Apr 30, 2018

Conversation

ara4n
Copy link
Member

@ara4n ara4n commented Apr 29, 2018

This is based directly on @walle303's PR at #1820, which I assumed was dead due to no feedback after a month. @walle303: thanks for doing all the initial legwork on this!

Reverts 8f778f5

Most of the discussion is on #1820; this extends the original PR to:

  • whitelist mime-types to prevent malicious ones being rendered by the browser
  • adds revokeObjectURLs to avoid leaking blobs
  • fixes the {} on the imports

This fixes element-hq/element-web#2678, element-hq/element-web#2866 (and many dups) and should generally speed up E2E a load.

@dbkr, PTAL - especially in whether my mimetype whitelisting idea is actually sound.

Based on @walle303's work at #1820
Deliberately reverts 8f778f5
Mitigates XSS by whitelisting the mime-types of the attachments so that malicious ones
should not be recognised and executed by the browser.
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I like the idea of whitelisting the mime types. As your comment says, the kicker is whether the browsers actually honour them, but they do seem to.

I still think we should create unqiue-origin object URLs in a sandboxed iframe once browser support allows, but I think this is safe enough for now.

@ara4n
Copy link
Member Author

ara4n commented Apr 30, 2018

woo, thanks for reviewing. have opened element-hq/element-web#6639 to track the migration to sandboxed iframes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disasterous performance on FF on rooms with large encrypted images
2 participants