Handlebars in attribute broken in Rails by CVE-2016-6316 fix

[dvandersluis]

I'm curious if anyone else has run into this problem. I updated Rails to 3.2.22.5, and suddenly started getting parse errors from ExecJS due to "s appearing in handlebars templates (I'm using hamlbars but I'm not sure if it matters here). After a couple days of frustration, I finally traced it to this change made in actionview to fix CVE-2016-6316. It was made in the 4.x and 5.x branches too, so my guess is that this actually affects all versions of rails > 3.

The problem manifests when a handlebars element is used as an attribute of an element created via tag or content_tag. For instance:

= content_tag :div, title: hb('t', key: 'general.title')

Before the Rails fix, this would create the following:

<div title="{{t key="general.title"}}"></div>

After the fix it creates:

<div title="{{t key=&quot;general.title&quot;}}"></div>

which then obviously doesn't parse properly, and ExecJS crashes on unexpected syntax.

In order to fix this, I monkey patched ActionView::Helpers::TagHelpers#tag_options to unescape the quote if it's between {{...}} or {{{...}}}:

module ActionView
  module Helpers
    module TagHelper
      module HandlebarsFix
        # When handlebars is passed into a button tag, ensure quotes aren't escaped within
        # This is needed because tag_options converts quotes to &quot entities, and then
        # when handlebars is compiled, parsing crashes.
        def tag_options(*)
          opts = super
          return unless opts

          opts.gsub(/\{{2,3}.*?(").*?\}{2,3}/) { |s| s.gsub('"', '"') }.html_safe
        end
      end
    end
  end
end

ActionView::Base.prepend ActionView::Helpers::TagHelper::HandlebarsFix
Haml::Helpers.prepend ActionView::Helpers::TagHelper::HandlebarsFix

I was wondering, though, if there's a way to fix this directly within handlebars_assets? My fix works for me, but I can see that there can be edgecases that might be eliminated if fixed directly at the source. I'm not exactly sure how parsing works in this gem, but maybe when a tag is found "s can be converted back to actual quotes?

[AlexRiedler]

I am really sorry I missed this when it happened; I don't regularly check the repo since it has been stable for quite some time.

I can't think of a way around it at this time without potentially causing an exploit. If it changed back the quot; then it might be exploitable. From here we don't actually have the attributes or parsing... hmmm I wonder how you can deem that string actually safe then... (maybe options to HAML or SLIM ???)