

Sanitising JSON callback identifiers for security — a must for web developers - tav
http://tav.espians.com/sanitising-jsonp-callback-identifiers-for-security.html

======
pilif
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.

~~~
tav
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!

~~~
tlrobinson
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.

~~~
tav
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...

~~~
tlrobinson
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.

~~~
tav
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?

------
DrewHintz
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...](http://friendfeed.com/api/feed/public?callback=alert%28document.cookie%29%3Bfoo)
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> ]

~~~
DrewHintz
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...](http://code.google.com/p/browsersec/wiki/Part2#Content_handling_mechanisms)

~~~
tav
Nice one Drew!

------
thwarted
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)".

~~~
tav
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... ?

~~~
thwarted
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.

