brianmario / escape_utils

Faster string escaping routines for your ruby apps
MIT License
513 stars 52 forks source link

Don't escape HTML in plain text emails #15

Closed kenn closed 2 years ago

kenn commented 13 years ago

I've found a similar ticket reported here, but it was closed without a fix so I'm re-raising the issue. (Rails 3.0.7 + Ruby 1.9.2)

https://github.com/brianmario/escape_utils/issues/14

In my case, not only '/' was escaped but other characters like single quotes got escaped too. So EscapeUtils.html_secure = false only fixed the url part of the problem.

Namely, with require 'escape_utils/html/erb':

Here's the link: http://www.example.com/

With require 'escape_utils/html/erb' and EscapeUtils.html_secure = false:

Here's the link: http://www.example.com/

By removing require 'escape_utils/html/erb', finally I got the correct result:

Here's the link: http://www.example.com/

Note that HTML tags like '<' or '>' aren't escaped in email templates with .text.erb extension, so it's not a Rails bug.

brianmario commented 13 years ago

Rails 3.0.7 will in fact escape the < and > tags (and the rest of what ERB::Util.html_escape escapes) by default in plain text emails. I was able to reproduce it fairly easily with a vanilla Rails 3.0.7 app and the following code https://gist.github.com/1001345.

Rails will always escape everything in any ERB code that aren't already marked html_safe before appending that data to the output buffer. To get around this, you can manually call the html_safe method on the string being returned. So in the case of my gist up there, you would do this instead:

<%= '<a href="http://www.testing.com">Testing</a>'.html_safe %>

Then that string would be appended to the output buffer unmodified.

Are you seeing different results from a vanilla Rails 3.0.7 app?

kenn commented 13 years ago

Ah right, the place I put < and > was outside of the ERB bracket - directly in the template, and confirmed that you're right in that point.

However, with the following template (FWIW the string actually comes from I18n.t in my case):

<%= "Here's the link: http://www.example.com/" %>

with require 'escape_utils/html/erb' and EscapeUtils.html_secure = false:

Here&#39;s the link: http://www.example.com/

without require 'escape_utils/html/erb':

Here's the link: http://www.example.com/

So at least something is wrong with the single quote.

brianmario commented 13 years ago

Yeah that's a difference with escape_utils vs ERB::Util.html_escape - escape_utils escapes the ' character while ERB's version does not. escape_utils follows the recommendations from the Open Web Application Security Project (owasp) at https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet#RULE_.231_-_HTML_Escape_Before_Inserting_Untrusted_Data_into_HTML_Element_Content

kenn commented 13 years ago

If I understand it correctly, the differnce between ERB::Util.html_escape vs escape_utils is that the former escapes 4 characters (&, <, > and ") whereas the latter escapes additional 2 characters (' and /).

And the EscapeUtils.html_secure option only gives you the control on /, thus the slight impedance mismatch. Why don't you have the same option takes care of both / and '? It not only makes sense (leaving single quotes is actually html unsafe :-), but makes the monkey patch even more transparent.

brianmario commented 13 years ago

Yeah I'd like to make it so the monkey-patches perform exactly like the libraries they patch. I'm so horribly behind on OSS work it's kindof ridiculous at this point :P

byroot commented 2 years ago

HTML escaping capabilities are deprecated in 1.3 because Ruby's CGI.escapeHTML is now faster.