Rails core - Nov 2010 - [patch] Let's use <%== %> instead of <%= raw() %>

I''ve submitted a small patch to make Rails behave properly with the
Erubis <%== %> construct. For some reason the current behaviour of
that tag in Rails 3 is to escape the contents _twice_ which is
probably a bug.

I offer three suggestions why this is a good idea:

- The syntax is cleaner. It can avoid a lot of .html_safe and raw in
your views. I especially like the conciseness of <%===
''<b>Alert</b>''
if level<0 %> better then the alternative with .html_safe.

- It performs slightly better since it saves a method call and we can
concat a String directly instead of coercing everything to a
SafeBuffer

- It re-enables the ability of Erubis to behave like Erb in Rails 2
which allows for easier upgrading (You can pass :escape => true to a
new Erubis instance or glabally replace the <%= with <%==)


https://rails.lighthouseapp.com/projects/8994/tickets/5918-fix-erubis-non-escaping-sequence-patch

It''s currently a monkey patch but if there is a chance of this being
accepted I can turn it into a proper patch with tests. Comments
welcome.

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On Fri, Nov 5, 2010 at 11:58 PM, Jan M. <jmfaber@gmail.com> wrote:
> I offer three suggestions why this is a good idea:
>
> - The syntax is cleaner. It can avoid a lot of .html_safe and raw in
> your views. I especially like the conciseness of <%===
''<b>Alert</b>''
> if level<0 %> better then the alternative with .html_safe.
The only concern I have is that the syntax here has subtly different
isn''t the same.  In erubis strings get escaped irrespective of whether
they''ve been escaped before, in rails it''s an idempotent
escaping
function so <%= h(x)  and <%= x are identical.

So if we override these additional operators we''ll be giving them our
own meaning, not increasing compatibility with erubis.  Having said
that, they''re completely broken now so I think this is probably a good
idea.

> - It performs slightly better since it saves a method call and we can
> concat a String directly instead of coercing everything to a
> SafeBuffer
We actually used to do something similar, but it was removed in this commit:

https://github.com/rails/rails/commit/4464b8e87bedd69816d4658c9386cc360affb62e

Was added here:

https://github.com/rails/rails/commit/209235165266ff39f2d14d02b497d7d703788104

Not sure whether that was intentional though.
> - It re-enables the ability of Erubis to behave like Erb in Rails 2
> which allows for easier upgrading (You can pass :escape => true to a
> new Erubis instance or glabally replace the <%= with <%==)
I don''t actually consider this a feature to be honest.  Yes
it''s
annoying having to mark strings as safe to prevent them being escaped,
but at least you notice it.  Contrast this with the previous behaviour
where things ''mostly worked'' until someone tried to attack
your site.
It''s annoying and fiddly, but compared to the alternative situation
where a single missing h can mean you give away root access to your
servers and all your customer''s passwords[1], I think the pain is
justified.

> It''s currently a monkey patch but if there is a chance of this
being
> accepted I can turn it into a proper patch with tests. Comments
> welcome.
I like the idea of making <%== work as a synonym for <%= raw, upload a
proper patch for it and I''ll take a look.



[1] http://blogs.apache.org/infra/entry/apache_org_04_09_2010

-- 
Cheers

Koz

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

Ok, I''ve uploaded a real patch (hope it''s OK, it''s my
first attempt at
using git).

Do you know the reason why the commits you mentioned above were
removed?

With regards to the .html_safe being "annoying but good for you":
that''s the same argument people use about static typing being
"annoying but good for you" and I don''t buy that argument
either. You
can still accidentally get non-escaped HTML in your code because of
string interpolation or because you marked an entire helper as safe.
The proper way to test for XSS scripting attacks is to put bad tags in
all fixtures and simulated user input and running the response body
though html tidy or similar. Similarly dynamic typing can be used by
responsible programmers by having appropriate tests.

Anyway, I''ve only changed the behaviour of the <%== tag and feeding
an
":escape => :off" option to Erubis can only be done by monkey
patching
Rails so that means a clueless programmer who needs to be protected
from himself won''t run into it quickly.


On Nov 5, 10:28 pm, Michael Koziarski <mich...@koziarski.com>
wrote:
> On Fri, Nov 5, 2010 at 11:58 PM, Jan M. <jmfa...@gmail.com> wrote:
> > I offer three suggestions why this is a good idea:
>
> > - The syntax is cleaner. It can avoid a lot of .html_safe and raw in
> > your views. I especially like the conciseness of <%===
''<b>Alert</b>''
> > if level<0 %> better then the alternative with .html_safe.
>
> The only concern I have is that the syntax here has subtly different
> isn''t the same.  In erubis strings get escaped irrespective of
whether
> they''ve been escaped before, in rails it''s an idempotent
escaping
> function so <%= h(x)  and <%= x are identical.
>
> So if we override these additional operators we''ll be giving them
our
> own meaning, not increasing compatibility with erubis.  Having said
> that, they''re completely broken now so I think this is probably a
good
> idea.
>
> > - It performs slightly better since it saves a method call and we can
> > concat a String directly instead of coercing everything to a
> > SafeBuffer
>
> We actually used to do something similar, but it was removed in this
commit:
>
> https://github.com/rails/rails/commit/4464b8e87bedd69816d4658c9386cc3...
>
> Was added here:
>
> https://github.com/rails/rails/commit/209235165266ff39f2d14d02b497d7d...
>
> Not sure whether that was intentional though.
>
> > - It re-enables the ability of Erubis to behave like Erb in Rails 2
> > which allows for easier upgrading (You can pass :escape => true to
a
> > new Erubis instance or glabally replace the <%= with <%==)
>
> I don''t actually consider this a feature to be honest.  Yes
it''s
> annoying having to mark strings as safe to prevent them being escaped,
> but at least you notice it.  Contrast this with the previous behaviour
> where things ''mostly worked'' until someone tried to
attack your site.
> It''s annoying and fiddly, but compared to the alternative
situation
> where a single missing h can mean you give away root access to your
> servers and all your customer''s passwords[1], I think the pain is
> justified.
>
> > It''s currently a monkey patch but if there is a chance of
this being
> > accepted I can turn it into a proper patch with tests. Comments
> > welcome.
>
> I like the idea of making <%== work as a synonym for <%= raw, upload
a
> proper patch for it and I''ll take a look.
>
> [1]http://blogs.apache.org/infra/entry/apache_org_04_09_2010
>
> --
> Cheers
>
> Koz
-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On 6 nov, 16:10, "Jan M." <jmfa...@gmail.com>
wrote:
>
> With regards to the .html_safe being "annoying but good for you":
> that''s the same argument people use about static typing being
> "annoying but good for you" and I don''t buy that
argument either.
Not really.

html_safe has nothing to do with static vs dynamic typing and
everything to do with whitelisting vs blacklisting as an approach to
security.

They''re completely different classes of issue.

(I won''t bother repeating what whitelisting and blacklisting are, nor
their relative merits, as it is well known.)

Cheers,
Wincent

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On Nov 7, 1:29 pm, Wincent Colaiuta <w...@wincent.com>
wrote:
> Not really.
>
> html_safe has nothing to do with static vs dynamic typing and
> everything to do with whitelisting vs blacklisting as an approach to
> security.
>
Not really. Comparing html_safe to whitelisting is not correct because
whitelisting provides an adequate protection and html_safe does not.
You will still need additional security tests because insecure things
can still get through with string interpolations. You could even claim
there is a danger that people are lulled into a false sense of
security with html_safe. And that is how I meant the analogy of static
typing versus dynamic typing: people claim that you should use static
typing because that will allow you to find bugs before they go into
production. That''s true to a point but it certainly does not mean that
you are somehow relieved of doing proper testing because you are using
a statically typed language. The analogy also works in reverse: if you
are doing proper testing than using a dynamically typed language is
not an issue, and similarly if you are testing the right way then it''s
OK to do the escaping yourself.


-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

> Not really. Comparing html_safe to whitelisting is not correct because
> whitelisting provides an adequate protection and html_safe does not.
Safe strings *are* whitelists, by calling html_safe on a string you
are instructing the erb handler that that string is to be considered
safe.  Every string which isn''t marked as safe, is considered unsafe
and is escaped.  That''s a whitelist.
> You will still need additional security tests because insecure things
> can still get through with string interpolations.
Newly interpolated strings aren''t safe until you mark them as such by
calling html_safe.
> and similarly if you are testing the right way then it''s
> OK to do the escaping yourself.
Similarly if you''re testing the right way you''ll notice that
your
values are being escaped unexpectedly and there''s no issue.

The erb handler is now secure by default, that''s a feature and the
minor pain it imposes is nothing compared to the danger of an XSS hole
in your application because your forgot that one character.


-- 
Cheers

Koz

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

That''s fine in theory but in the last few weeks of using Rails 3
I''ve
still noticed unescaped HTML slipping though because so many html_safe
statements are needed that you make mistakes, or you mark an entire
helper safe that gets it content from somewhere else, or you marked a
string safe and then interpolated an unsafe item into it.

Anyway, it''s all besides the point because as I started out saying: I
am NOT against on-by-default escaping, I just think there is no reason
not to provide an elegant way to use unescaped text. You still need
unescaped text, for example when you are generating a plain text e-
mail. My e-mail templates look much cleaner with <%== everywhere
instead of all the html_safe stuff.


On Nov 7, 9:22 pm, Michael Koziarski <mich...@koziarski.com>
wrote:
> > Not really. Comparing html_safe to whitelisting is not correct because
> > whitelisting provides an adequate protection and html_safe does not.
>
> Safe strings *are* whitelists, by calling html_safe on a string you
> are instructing the erb handler that that string is to be considered
> safe.  Every string which isn''t marked as safe, is considered
unsafe
> and is escaped.  That''s a whitelist.
>
> > You will still need additional security tests because insecure things
> > can still get through with string interpolations.
>
> Newly interpolated strings aren''t safe until you mark them as such
by
> calling html_safe.
>
> > and similarly if you are testing the right way then it''s
> > OK to do the escaping yourself.
>
> Similarly if you''re testing the right way you''ll notice
that your
> values are being escaped unexpectedly and there''s no issue.
>
> The erb handler is now secure by default, that''s a feature and the
> minor pain it imposes is nothing compared to the danger of an XSS hole
> in your application because your forgot that one character.
>
> --
> Cheers
>
> Koz
-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

> Anyway, it''s all besides the point because as I started out
saying: I
> am NOT against on-by-default escaping, I just think there is no reason
> not to provide an elegant way to use unescaped text.
Agreed, I''ve applied it to master and 3-0-stable :).


-- 
Cheers

Koz

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

Thanx! On my first patch no less!
Cheers,
Jan

On Nov 7, 10:19 pm, Michael Koziarski <mich...@koziarski.com>
wrote:
> > Anyway, it''s all besides the point because as I started out
saying: I
> > am NOT against on-by-default escaping, I just think there is no reason
> > not to provide an elegant way to use unescaped text.
>
> Agreed, I''ve applied it to master and 3-0-stable :).
>
> --
> Cheers
>
> Koz
-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On Sun, Nov 7, 2010 at 11:29 PM, Jan M. <jmfaber@gmail.com> wrote:
> Thanx! On my first patch no less!
Welcome :)

http://contributors.rubyonrails.org/contributors/jan-maurits-faber/commits

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.