#32 ✓invalid
Domizio Demichelis

The translation helper is unsafe!

Reported by Domizio Demichelis | November 23rd, 2010 @ 01:33 PM

(ActionView::Helpers::TranslationHelper.translate)

If a translation key has the suffix "_html or the last element of the key is the word "html" the returned string is marked as safe_html.

That is unsafe because the interpolated variables might be unsafe an the helper is marking them as safe without escaping.

Example:

your_key_html: "<b>This is a</b> %{unsafe}  output because it is %{something} against the %{rails} policy"

When you use it in possible many place in the application you have to remember that you must manually escape all the variables all the time you use it, that's because the helper will mark the translated string as safe, regardless to what is contained (the html_safe? status) in the interpolated variables.

If you use the translation of the example in 10 places, you will have to remember to escape 30 variables, and that is not only silly but is a main source of problems. Besides it is specially bad because it works against the new rails policy, that allows you to forget about security and just have everything escaped automatically. IMHO that is a mayor breach of security.

Every helper that produce html MUST escape the (potentially unsafe) input, adding its own html (which is obviously safe) and mark its output as safe. That way the input that contains html marked as safe will not be escaped (because the h(variable) mark for escape only what is not marked as safe).

We need to keep the html suffix as a convention to mark the base translation string as safe BUT the interpolated variables MUST be escaped, AND the output must be always marked as safe. In case the base translation string is not marked as safe (not "html" suffixed) it must be escaped as well, AND the output must be marked as safe.

In conclusion the helper must always return a safe_html string, escaping the base translation string when its key is not suffixed with "html" and must escape all the interpolated variable.

If you agree with the principle, I could write a patch for that. Please, let me know. Thank you.

Comments and changes to this ticket

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

Repository for collecting Locale data for Ruby on Rails I18n as well as other interesting, Rails related I18n stuff

People watching this ticket

Attachments

Tags

Pages