#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

  • Sven Fuchs

    Sven Fuchs December 26th, 2010 @ 05:58 PM

    • State changed from “new” to “invalid”

    Hi ddnexus,

    your approach sounds good to me. I think this lighthouse is the wrong place to report though because the translation helper is part of Rails.

    Could you reopen the ticket over at Rails' lighthouse and provide a patch?

    Thanks a lot! :)

    https://rails.lighthouseapp.com/

    Will close the ticket here.

  • murphy (at rubychan)

    murphy (at rubychan) November 4th, 2011 @ 12:07 PM

    Hi ddnexus and Sven!

    Apparently, the issue never got fixed in Rails 2.3 or later.

    I couldn't find any corresponding ticket in the Rails project, so I will create a new one.

    In any case, here's a simple patch for 2.3:

    # abandoned ticket from 2010-11-23: http://i18n.lighthouseapp.com/projects/14948/tickets/32-the-translation-helper-is-unsafe
    module ActionView::Helpers::TranslationHelper
      def translate_with_html_safe_options(keys, options = {})
        if !options.empty? && keys =~ /(\b|_|\.)html$/
          options = {}.tap do |html_safe_options|
            for name, value in options
              html_safe_options[name] = h(value)
            end
          end
        end
        
        translate_without_html_safe_options keys, options
      end
      alias t_with_html_safe_options translate_with_html_safe_options
      
      alias_method_chain :translate, :html_safe_options
      alias_method_chain :t,         :html_safe_options
    end
    

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

Pages