TLDR
Transactional emails are vulnerable to HTML injections. There should be a workspace-wide switch to control whether variables are HTML-escaped by default to avoid this.
Full message
Greetings,
One of the things I love about CIO is how the rich visual editor integrates seamlessly with code. However, a few months ago, a pentest carried against my app uncovered a nasty HTML injection vulnerability caused by default behaviour of transactional emails.
Take the following template:
Hello {{trigger.userName}},
You’ve been added as an administrator of the {{trigger.workspace}} team.
- The ACME team
This transactional email is to be sent from our backend with proper values for `userName` and `workspace`. As simple as this may look, this template is vulnerable to HTML injection if either variable includes HTML! In this example, an attacker could craft a special value for either the user’s name or the workspace name, and send phishing emails originating from my company’s email address to my other users! Not good.
I am aware of the `escape` Liquid operator (e.g., `{{trigger.workspace | escape}}`) and of the possibility to escape the variables in the backend before sending them to CIO. While this works as a preventative measure, the real problem here is that the current setup is insecure-by-default. Developers need to ensure pro-actively, for each transactional template, that no unescaped variables are sent.
This is a problem because, firstly, many devs (including me, prior to the pentest) are probably not even aware of this latent vulnerability, and secondly, this vulnerability needs to be considered every time a transactional template is created or modified, which is easy to forget and to overlook.
As full-stack development matures, many frameworks and technologies are now designed with secure-by-default principles, for example:
- React automatically HTML-escapes all variables (e.g., `<div>Hello, {user.name}!</div>`) so developers don’t need to worry about HTML injection. To circumvent this, the developer needs to instruct React specifically not to HTML-escape.
- Most modern ORMs automatically SQL-escape variables (e.g., `SELECT * FROM users WHERE id = ${userId}`), so developers don’t need to worry about SQL injection for every query they write.
In light of the above, I would suggest adding a workspace-wide setting which determines whether or not variables are HTML-escaped by default. If the setting is activated, then all variables are automatically HTML-escaped, unless a special Liquid operator is passed, for example `{{trigger.my_dynamic_html_content | unsafe_html}}`. To maintain current behaviour, the switch could be off by default for existing accounts.
Best,
Tomer