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

Escape translation context variables #78

Closed
janezkranjc opened this issue Oct 15, 2018 · 10 comments
Closed

Escape translation context variables #78

janezkranjc opened this issue Oct 15, 2018 · 10 comments

Comments

@janezkranjc
Copy link

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
Copy link
Author

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

@kemar
Copy link
Contributor

kemar commented Oct 16, 2018

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
Copy link
Author

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 kemar closed this as completed in 44e409d Oct 17, 2018
@kemar
Copy link
Contributor

kemar commented Oct 17, 2018

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
Copy link
Contributor

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

image

@majestic2k
Copy link
Contributor

@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
Copy link
Contributor

kemar commented Jan 4, 2019

@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
Copy link
Contributor

majestic2k commented Jan 4, 2019

@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
Copy link
Contributor

kemar commented Jan 5, 2019

@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 kemar reopened this Jan 5, 2019
@kemar kemar closed this as completed in 1b44cc3 Jan 5, 2019
@kemar
Copy link
Contributor

kemar commented Jan 5, 2019

@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>

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

4 participants