JSON encoder shouldn't encode "<" and ">" as octals but as escaped unicode char
chuyeow
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
Michael Koziarski
> 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 ;).
--
Cheers
Koz
Danny Burkes
> 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
--
Posted via http://www.ruby-forum.com/.
Danny Burkes
> http://dev.rubyonrails.org/changeset/6893, which caused this problem,
> was contrary to that philosophy.
>
Damn encoder :-) That's
http://dev.rubyonrails.org/changeset/6893
- D
Michael Koziarski
> 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 ;)
--
Cheers
Koz
Danny Burkes
> unless this change will also address the security issue identified,
> it's a complete non-starter ;)
>
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
Michael Koziarski
> 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 :)
--
Cheers
Koz
Rick Olson
> 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 :)
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...
Danny Burkes
> 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.
Rick Olson
> 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?
--
Rick Olson
http://lighthouseapp.com
http://weblog.techno-weenie.net
http://mephistoblog.com
Danny Burkes
>
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?
Rick Olson
>
> > 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?
Seems to work for me. I tried exploiting it and all that too, but the
browser wasn't having any of that. Anyone else?
Hugh Messenger
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.
From my reading of http://dev.rubyonrails.org/ticket/9975 it 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?
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
DHH
> 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:
Your team can probably just inline patch that if they're on a recent
enough version of rails.
Danny Burkes
> This has been fixed. Here's the change that actually does the work:
>
Outstanding- thanks for letting us know.
- D
Hugh Messenger
> Your team can probably just inline patch that if they're on a recent
> enough version of rails.
Done, and working.
Many thanks.