Serializing configuration to client should do the proper encoding
imalberto opened this issue · 9 comments
See Trello card 745 for more info
Here is the implementation used in Mojito today:
https://github.com/yahoo/mojito/blob/develop/lib/app/autoload/util.common.js#L105
And here is where we clean up YUI_config before sending it to the client side:
https://github.com/yahoo/mojito/blob/develop/lib/app/addons/ac/deploy.server.js#L127
Can you guys point to some documentation that explains the attack vector? An authoritative blog post would do — I want to have something to link to in the code and HISTORY.md, and also have an example to test.
There are also other issues related with the encoding. Like this one:
The encoding was originally put in place in response to bugzilla ticket 5590319 (marked as S1).
Comment 10 of that bug has a valid (in my mind) question which no one seemed to answer (in the bug log):
"Where does this config data come from, is it locally sourced form the machine, or does it come form the browser?"
Encoding was added in commit YahooArchive/mojito@83a68da453.
Yeah, the reality was that cofnig was coming from a merge between the locally sourced from the machine + any custom config produced by controllers (which could include request data). That's not longer the case for configs, but it is still a valid point for any data pushed thru the data channels in mojito. Which means that this encoding is still a very valid concern. I will probably recommend to add encoding by default in express-state for any res.expose() call, unless the specific call is flagged somehow. While app.expose() is probably fine with the encoding disabled by default since it happens before requests arrive (in most cases).
While
app.expose()is probably fine with the encoding disabled by default since it happens before requests arrive (in most cases).
I won't differentiate between the app vs. res expose() calls because it all gets grouped together and it's easy to call req.app.expose() (not sure why you would, but very easy), also I'll do the encoding at serialization time which means both the app- and response- level data will get encoded.
@caridy I wonder if the solution to fixing escaped unicode chars in URLs (YahooArchive/mojito#1257) is to use HTML entities like YUI's Y.Escape.html() function uses: http://yuilibrary.com/yui/docs/api/files/escape_js_escape.js.html#l10
Alright, once we start using express-state in mojito, we can remove a lot of code :)