Possible remote code execution?

[niccokunzmann]

Context
I just stumbled upon this email in crowdin. Here is a snippet:

<% for(var i=0; i<tickets.length; i++) { %>
<%= tickets[i].ticketName %> <%= tickets[i].ticketType %> <%= tickets[i].quantity %>

My guess
This could be Java or JavaScript or C++. I think it is JavaScript because the server is a node server. I am not sure if the code above is a special, restricted code of a view. It could be that it runs directly on the model.

Issue 1
If this code runs unprotected, it is a security hole that translators can exploit. Arbitrary code can be injected. I worry about a possible security hole.

Issue 2 I am not sure about how translators from a non-coding background perceive this text. Something like <%user.name%> marks something untranslatable. I worry that putting code into crowdin reduces the number of people willing to translate the text with the code.

Questions

These are questions I have.

• [x] Is this some restricted code like Jinja2? Is this code a security risk?
• [ ] Should code be translated to different languages? If no - should it be translatable?

[Wardormeur]

Mh, from what i believe it is, it's hardly spoofable as the data comes from the backend. However, anybody "could" replace this bit of code by .. anything if they wanted to, in a way translation are "open", to a point a random link could be hardcoded into the translation, which means verification is the translation is crucial, but hard for us to do as we're not (native, or not at all) speakers. I'm going though to look over what kind of templating engine is used, i sincerly don't know at that point, it's abstracted for us by node-mailer. On a translation perspective, we can't remove variables :) However, as much as possible, we could probably reduce to a single depth of data (when possible) in a way user.name becomes username. The other solution is to multiply the maount of text and "compose" the emails, based upon multiple strings and post-processing for variable injection. That will require some work, and string will loose their context. Not sure if that's something we want

[Wardormeur]

Regarding issue 2, we need to find a better solution. I've been stumbling upon translations that are .. translating the variables/changing the processing tags First problem : crowdin recommend to add a space after an ".", which means we need to flatten in order to reduce the error possibility. I'll talk with @PhilipHarney to see if we can "freeze" some parts of the translations to avoid changing anything between <% %> If we can't, we'll have to rethink the whole process in order to not expose it.

[Wardormeur]

Ah, well, this issue was old, but we made some progress. We added a security during CI to test if the tokens (<% a_string %>) are the one expected. (https://github.com/CoderDojo/cp-translations/commit/6bfe32219023dc8f7be8d99b6a3fcabc42bb1b1d) It's not perfect (I bet you can find a way to bypass the tokenizer), but like everything, it's a cost/time spent vector. I think we reached the limit of what can be done here, apart if you have another idea?