JSON encoder shouldn't encode "<" and ">" as octals but as escaped unicode char

As far as I can tell the '<' and '>' characters don't actually need to
be escaped to be valid JSON, but it's done anyway in Rails since
PrototypeHelper and JavaScriptHelper depend on it, so I think we
should leave it as it is until we decouple JSON encoding from the view
helpers for generating inline JavaScript.

So, back to the issue at hand, which is that the string encoder for
ActiveSupport::JSON converts "<" and ">" to '\074' and '\076', when it
should actually be encoding them as '\u003C' and '\u003E'.

Please help verify (please comment with +1 if it works for you, it'll
help get this patch into Rails sooner!) or add any comments you have
here: http://dev.rubyonrails.org/ticket/9975

Thanks for reading!

Cheers,
Chu Yeow

As far as I can tell the '<' and '>' characters don't actually need to
be escaped to be valid JSON, but it's done anyway in Rails since
PrototypeHelper and JavaScriptHelper depend on it, so I think we
should leave it as it is until we decouple JSON encoding from the view
helpers for generating inline JavaScript.

This was done to fix a XSS vulnerability (CVE-2007-3227).

So, back to the issue at hand, which is that the string encoder for
ActiveSupport::JSON converts "<" and ">" to '\074' and '\076', when it
should actually be encoding them as '\u003C' and '\u003E'.

I assume that browsers still do the 'right' thing with those values?
I'm all for valid json, but not at the expense of security ;).

So, back to the issue at hand, which is that the string encoder for
ActiveSupport::JSON converts "<" and ">" to '\074' and '\076', when it
should actually be encoding them as '\u003C' and '\u003E'.

+1

If the current core philosophy is that to_json should always generate
spec-valid JSON (as evidenced by
http://dev.rubyonrails.org/changeset/7697), then
http://dev.rubyonrails.org/changeset/6893, which caused this problem,
was contrary to that philosophy.

- Danny

http://dev.rubyonrails.org/changeset/7697), then
http://dev.rubyonrails.org/changeset/6893, which caused this problem,
was contrary to that philosophy.

Damn encoder :slight_smile: That's

http://dev.rubyonrails.org/changeset/6893

- D

If the current core philosophy is that to_json should always generate
spec-valid JSON (as evidenced by
http://dev.rubyonrails.org/changeset/7697), then
http://dev.rubyonrails.org/changeset/6893, which caused this problem,
was contrary to that philosophy.

You're welcome to whatever philosophy you choose to hold to, but
unless this change will also address the security issue identified,
it's a complete non-starter :wink:

You're welcome to whatever philosophy you choose to hold to, but
unless this change will also address the security issue identified,
it's a complete non-starter :wink:

Of course < and > should be encoded- but they should be encoded per the
spec, as \u003C' and '\u003E', respectively. They are currently encoded
as '\074' and '\076', which doesn't comply with any JSON specification
that I'm aware of.

All I'm advocating for is valid JSON- I thought that was the Core team's
position now as well, since they finally addressed the key-quoting issue
in http://dev.rubyonrails.org/changeset/7697

Am I wrong about that?

- D

All I'm advocating for is valid JSON- I thought that was the Core team's
position now as well, since they finally addressed the key-quoting issue
in http://dev.rubyonrails.org/changeset/7697

Am I wrong about that?

No, you're not wrong. All I'm looking for is for people to report
that the change does still address the XSS vulnerability when they +1
the ticket. The change seems perfectly reasonable, but the
vulnerability was the result of the browsers' liberal parsing
algorithms doing things which still seem unreasonable to me, who
knows if this change still fixes it?

We don't need more talk of validity or spec compliance, we need
reports about browser behaviour from multiple different platforms and
browsers. The secunia guys have an advisory about this particular
issue, I suppose they'll be able to let you know if the change still
triggers their test script.

If we can be comfortable we're not introduce a security regression,
then we can down to the talk about how we encode those values,
whether it's sane to assume utf-8 encoded strings, and all that other
good stuff :slight_smile:

If we can be comfortable we're not introduce a security regression,
then we can down to the talk about how we encode those values,
whether it's sane to assume utf-8 encoded strings, and all that other
good stuff :slight_smile:

One thing to consider, too, is that this only affects JSON posted on a
web page, not sent between requests. Perhaps there's something like
the html_encode helper for JSON:

<%= j @foo.to_json %>

Just throwing ideas out...

One thing to consider, too, is that this only affects JSON posted on a
web page, not sent between requests. Perhaps there's something like
the html_encode helper for JSON:

<%= j @foo.to_json %>

I like that- it's worth mentioning that the non-compliance issue I'm
having is with non-browsers choking on the to_json output, not browsers.
So distinguishing between the two seems like a good compromise, if we
need one.

- D

I like that- it's worth mentioning that the non-compliance issue I'm
having is with non-browsers choking on the to_json output, not browsers.
So distinguishing between the two seems like a good compromise, if we
need one.

I don't... But we don't do that for HTML so I'm not sure we have to
for JSON (and I think it's pretty rare that JSON is in HTML anyway).
Do you wanna whip up a patch with the right UTF codes?

Do you wanna whip up a patch with the right UTF codes?

Sure, happy to do it, but is there something wrong with the latest patch
attached to http://dev.rubyonrails.org/ticket/9975 ? Does it go a bit
too far?

- D

Seems to work for me. I tried exploiting it and all that too, but the
browser wasn't having any of that. Anyone else?

Where are you guys at with this?

I don't code with RoR, I'm working in PHP processing JSON from someone
elses RoR API ... and y'alls octal encoding of < and > is breaking PHP
5's json_decode(), which returns NULL for anything with these encodings
in it. I googled into this thread researching the problem.

this may be fixed, with the correct UTF encoding? If so, should it be
simple for the dev team building the RoR RAPI I'm accessing to apply the
patch?

Sorry if these are obvious questions, I'm just trying to make sure RoR
and PHP can play nicely in the same sandbox. Obviously I can
pre-process my JSON responses to correct the problem, but it'd be nice
if RoR could adhere to the JSON standard (such as it is!).

   -- hugh

From my reading ofhttp://dev.rubyonrails.org/ticket/9975it seems like
this may be fixed, with the correct UTF encoding? If so, should it be
simple for the dev team building the RoR RAPI I'm accessing to apply the
patch?

This has been fixed. Here's the change that actually does the work:

http://dev.rubyonrails.org/changeset?new=trunk%2Factivesupport%2Flib%2Factive_support%2Fjson%2Fencoders%2Fstring.rb%408050&old=trunk%2Factivesupport%2Flib%2Factive_support%2Fjson%2Fencoders%2Fstring.rb%408026

Your team can probably just inline patch that if they're on a recent
enough version of rails.

DHH wrote:

This has been fixed. Here's the change that actually does the work:

Outstanding- thanks for letting us know.

- D

DHH wrote:

Your team can probably just inline patch that if they're on a recent
enough version of rails.

Done, and working.

Many thanks.

   -- hugh