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

XSS vulnerability on ProtonMail web client #634

Closed
rikako3 opened this issue Sep 8, 2023 · 3 comments
Closed

XSS vulnerability on ProtonMail web client #634

rikako3 opened this issue Sep 8, 2023 · 3 comments

Comments

@rikako3
Copy link

rikako3 commented Sep 8, 2023

https://www.sonarsource.com/blog/code-vulnerabilities-leak-emails-in-proton-mail/

A few days ago, Sonar published a blog post about a cross-site scripting vulnerability in the ProtonMail web client that allowed attackers to execute arbitrary JavaScript in the context of the top frame. This allows stealing decrypted emails or impersonating the victim by sending emails.

According to the timeline from the blog post, Proton fixed this on the public repo on June 15 and they deployed it to production on July 6.

Does this vulnerability affect ElectronMail? Since the last release is from June 10, I think the upstream fix from June 15 is not included, but I'm not sure if the application prevents this somehow.

In case ElectronMail is also affected, I would like to ask if there is any possibility to implement some mitigation or configuration option to prevent XSS in case there is another vulnerability in the future. The Sonar blog post lists 3 recommendations to avoid sanitizer bypasses, but those look like they'd need to be implemented upstream. Maybe some kind of "plain text view" option could be added in order to read emails without having to parse any HTML, or without executing scripts?

Thank you for your time.

@vladimiry
Copy link
Owner

vladimiry commented Sep 8, 2023

  • The wip-5.2.0 branch includes updated/week-old ProtonMail web client. So if you wish, you can assemble a fresh app package from sources.
  • The Advanced Options block, located on the account edit form, contains Block non "API entry point"-based network requests toggle. See some screenshots in Use DuckDuckGo's Image Proxy #312. It's a sort of firewall thing which aimed to protect from a third-party requests. So even if XSS script gets injected and executed, it's going to face issues with sending stolen data somewhere expect the whitelisted Proton domain addresses. By the way, this feature helped to detect privacy issue in ProtonMail web client v4-beta with consequent bug bounty award.
  • The app's local store mode displays email messages in the text format inside an iframe, the sandboxing + CSP rules are applied to which (these measures mentioned in referenced Sonar article). Before displaying as put above, the email body content gets transformed to a text via sanitize-html module. Besides, this "local store" mode is functional in offline. So if you prefer, you could read the email with a network connection turned off.

@rikako3
Copy link
Author

rikako3 commented Sep 8, 2023

I see, thanks for the explanation. Both the Block non "API entry point"-based network requests toggle and the sanitized view in the local store are exactly the kind of features I was asking for.

Just a question: have you considered using DOMPurify instead of sanitize-html? The developer seems to have more focus on security (it has a bug bounty), it's more actively maintained and has no dependencies.

Thank you again for your work, ElectronMail is really a great application.

@vladimiry
Copy link
Owner

vladimiry commented Sep 8, 2023

Just a question: have you considered using DOMPurify instead of sanitize-html? The developer seems to have more focus on security (it has a bug bounty), it's more actively maintained and has no dependencies.

DOMPurify works in a browser scope (renderer process in terms of @electron), but the current app design needs the library to be functional in the node.js scope, since sanitizing task being performed in the @electron main process, so the renderer process receives already sanitized data. It's possible to redesign it, but there is no such goal at the moment.

@rikako3 rikako3 closed this as completed Sep 8, 2023
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

No branches or pull requests

2 participants