Sanitising JSON callback identifiers for security — a must for web developers | Hacker News Reader

Sanitising JSON callback identifiers for security — a must for web developers

tav.espians.com

18 pointstav15 comments

cross-posted from the comments there:

the problem here is the fact that whatever you pass to friendfeed is again executed in the context of your page.

This means that the alert() you so "cleverly" injected runs in the context of the CALLERs page, so the document.cookies that are alerted are the cookies on the source page (the page where the friendfeed-page is sourced on), not of the target page (friendfeed).

This also means that friendfeed doesn't have to do sanitization because whatever you are "injecting" there you could as well just put on your page to begin with.

The real problem behind JSONP is that the consumer has to trust the provider and there is no way to do ANY sanitization of that (the moment you add that "script src=" you are toast and you need to do that because requesting the script via XHR for example isn't possible due to the same origin policy).

So while this might look like a nice idea, it's completely unneeded and doesn't solve the real problem, which is, unfortunately, insolvable in pure JavaScript. You can of course proxy the connection, but not having to do that is what JSONP was invented for.

Thanks, I was puzzling over whether I was missing something, since the callback itself can run arbitrary code.

I actually use this technique with BOSS to store the original search query parameters since they're not always possible to recover from the results object, eg:

var callback = 'YBOSS.setLastQuery(\''+quotedQuery+'\');YBOSS.retrieved';

also, cross-posted from the comments there:

Hey Philip,

You are right, the code is executed in the context of the CALLERs page. But that is the problem...

Imagine the scenario, where a client-side web application allows arbitrary data sources to be added... it's not too implausible a future where instead of adding RSS feeds to an RSS Reader, one adds JSON feeds to such an application...

In this context, if the user is tricked into adding a maliciously crafted URL, then everything from their identity to their data is accessible... not to mention being able to abuse the account to spread a worm even...

I believe the possibilities of using JSONP haven't been explored much yet, and for it to go further in the context of interesting mashups/applications, it's important that it not be a security hole...

Hope that makes sense!

I still don't understand the problem.

In your use-case a user provides your application with a URL to a JSONP API, which they obtained from someone malicious who wants to steal the user's data (from your application, or from the JSONP source?)

Presumably the URL contains the malicious callback parameter, since that's what you're sanitizing, but your application needs to provide it's own callback parameter to get the resulting JSON. Shouldn't it strip the entire callback parameter and add it's own?

IMO the much bigger problem in this scenario is that you're loading untrusted URLs in script tags. If an attacker is able to trick the user into entering a URL with a malicious callback parameter, what's stopping them from using a URL (with or without a callback) pointing to a file on their own server which can contain any JavaScript they like.

Sure, that's why one should only add hosts/base urls which one trusts, e.g.

  http://github.com/api/json/
  http://api.twitter.com/json/

But with what you suggest, then you would've to keep a lot more state -- i.e. knowing beforehand every single possible URL the host may offer.

Whilst that would be a solution, it also slows down development dramatically. What if Twitter were to rename/add APIs?

Being able to trust certain hosts/base urls makes life much easier and lets things evolve at a much faster pace...

Ok, fair enough, that addresses my last paragraph.

But I still don't see what problem this sanitization solves. For a JSONP source to be useful you need to specify your own callback, not rely on one provided by a user input. Shouldn't you strip the callback and add your own?

Furthermore, even if there was a reason to keep the user specified callbacks, shouldn't the application that's consuming the URLs do the sanitization, not the JSONP provider?

It's your responsibility to secure your app, not the 3rd party JSONP providers.

Good points. I was imagining a scenario in which users/other-app-providers would be able to define their own Javascript functions, Google Caja http://code.google.com/p/google-caja/ style... in which case, the app should simply only need to whitelist adding trusted hosts/base urls to <script src="" />...

Also, see Drew's comment below where he was able to find an XSS exploit in FriendFeed due to them accepting '<' and '>' in the callback value... a reflection of how most security exploits come from combining different vulnerabilities.

Hope I make sense?

And exactly how are checks in legitimate source going to solve issue with tricking users to add malicious sources?

Whitelist the trusted sources =)

As pointed out here and in the blog's comments, if example.com does a <script src="http://friendfeed.com/api/feed/public?callback=alert%28docum... the code will run in example.com's origin, not friendfeed.com's. This means it's not an XSS in FriendFeed. When testing for this type of issue, using alert(document.domain) makes it clear what origin the JavaScript is running in.

Although the blog did not fully describe a real XSS, developers should not allow characters such as < and > in JSONP callbacks.

Does anyone know how to contact security at FriendFeed? [update: there's a bug report form, hopefully someone reads it http://friendfeed.com/about/contact/bug ]

Reporting a bug through their bug report form worked. :)

FriendFeed has fixed the bug. There was a way to exploit this now-fixed issue as actual XSS in FriendFeed for certain browsers. http://code.google.com/p/browsersec/wiki/Part2#Content_handl...

Nice one Drew!

Ignoring what pilif said (which is all valid), the problem with accepting any of

  <identifier>
  <identifier>.<identifier>
  <identifier>[<integer>]

is that the choice of just these three is kind of arbitrary. Really, you're whitelisting a specific subset of "expression" that presumably returns a function to be called. Why not allow a function that returns a function? Or an object property references as object[string]? Or nested object.property.property expressions, or an arbitrarily complex expression? It's much easier, if you're going to actually limit it, to just accept a single string and have the calling page be sane enough to make sure that name is in scope and resolves to a callable function, rather than making the whole thing harder on yourself trying to figure out exactly which expressions are "safe(st)".

If I understand it right, you're suggesting limiting it to just an <identifier> ? I agree and if we would all standardise on just that it'd fine by me...

But, having said that, array_of_functions[index] is also a common pattern that i've personally had to resort to several times... and is also defined in the original JSON-P article... The code already supports nested object.property.property expressions and I'd welcome patches to support object['key'] -- which I've also used several times in the past.

And given the very little extra effort (less than 10 lines of code) required to support:

<identifier>.<identifier> <identifier>[<integer>]

I didn't see why/how it'd hurt to put that forward to standardise around... ?

Because it's (relatively) easy to validate (if you're going to) a simple identifier (ignoring UTF-8 issues and that $ can be in an identifer, I'd almost consider it matching ^\w+$ to be enough), and it's increasingly harder to properly validate javascript expressions without actually having a javascript interpreter and parsing/executing it. More complexity means an incrase in the possibility of bugs and more divergent implementations.

Reply on news.ycombinator.com