Polyconseil / vue-gettext

Translate your Vue.js applications with gettext.
MIT License
278 stars 55 forks source link

Escape translation context variables #78

Closed janezkranjc closed 5 years ago

janezkranjc commented 6 years ago

Right now if I do this:

<span v-translate="{myVar:var}">This is my variable %{myVar}</span>

If the value of var is an unescaped HTML entity such as <br> the resulting HTML will be:

<span>This is my variable <br></span>

In contrast if I do this without v-translate the code looks like this:

<span>This is my variable {{var}}</span>

And the resulting HTML is:

<span>This is my variable &lt;br&gt;</span>

I believe the expected result with using v-translate should be the same.

janezkranjc commented 6 years ago

This becomes also a security issue if a variable is malicious code such as a <script> tag, which gets executed.

kemar commented 6 years ago

Sometimes you have to embed HTML in your translations, it's a valid use case. The directive supports HTML content, it's a built-in feature, see the documentation: https://github.com/Polyconseil/vue-gettext#html-support-difference-between-the-component-and-the-directive

Escaping HTML in the directive would break the current behaviour.

Right now, if you want to avoid the rendering of HTML content, you should use only the component.

Note for later: sanitization of HTML in the directive could be a security improvement.

janezkranjc commented 6 years ago

I don't want to escape HTML inside the directive but inside the variables that are evaluated which is what vue does with handlebars. Or am I wrong?

kemar commented 6 years ago

Nope you're right! Thanx for the report. I published a new version with a fix https://github.com/Polyconseil/vue-gettext/releases/tag/v2.1.1

narrowtux commented 6 years ago

This breaks all the translate tags! now it looks like this:

image

majestic2k commented 5 years ago

@kemar first of all thanks for the library, but you've introduced breaking change with this fix in a minor release.

v-translate was the only way to hide HTML from translator and be able to change parts of html without breaking translation keys.

Real world example: I want to hide css classes from translator (so i can change tags or their classes without retranslation), so i wrap some tags with classes in a variable.

<template>
    <p v-translate="{accentStart, accentEnd, percent: 10}">Add %{accentStart}%{percent}%%{accentEnd} experience.</p>
</template>

<script>
export default {
    data() {
        return {
            accentStart: '<span class="accent__secondary">',
            accentEnd: '</span>'
        };
    }
}
</script>

After this fix it's impossible to do that.

kemar commented 5 years ago

@majesticcpan yes I can see 😢 Thank you very much for your example.

But, and I'm sorry for that, I can't revert nor cancel this commit because it solves a major XSS vulnerability. Dynamically rendering arbitrary HTML can be very dangerous for your app.

Given your use case, I prefer to make life a little less pleasant for translators, but very more secure for your users.

majestic2k commented 5 years ago

@kemar, i've understood your concerns about XSS, but that feature is really needed if your codebase is "external data free", and you have 40+ translations, when every change of a translation key is painful. I really don't want to fork lib or something like this. Is there a chance that we can introduce some global option to allow html in variables or another way to do that (maybe as an option of a directive, or if an object is passed as a value to variable, so we can detect that specific variable can be unescaped)? (if so, i'll try to make a pull request)

kemar commented 5 years ago

@majesticcpan ok I understand your point. A directive's option could be the best way to solve this. I'll work on it this week-end.

kemar commented 5 years ago

@majesticcpan, there is a new attribute for directives in release 2.1.2 that should solve your problem.

I updated the documentation here:

<p
  v-translate
  render-html="true"
  >
  Hello %{ openingTag }%{ name }%{ closingTag }
</p>