tc39 / proposal-cancelable-promises

Former home of the now-withdrawn cancelable promises proposal for JavaScript
Other
376 stars 29 forks source link

Potential security issues when applied to fetch() #4

Closed annevk closed 8 years ago

annevk commented 9 years ago

I think this is what @sleevi was alluding to the other day on Twitter.

An attacker that is in control of canceling a "no-cors" fetch can influence how many bytes end up in the body. This might result in some crucial bit of the resource not ending up with the user.

Perhaps we should enforce Content-Length or some such and result in a network error in cases like this? Streaming doesn't work with "no-cors" (opaque) so that's less of a concern (though that has its own set of concerns).

sleevi commented 9 years ago

There's a variety of attacks; sorry I left it alluded to - mostly it was needing to sit down with @jakearchibald in person to prove I'm not a horrible person who hates everything (or at least, I do hate everything but can be reasoned with)

I think there are a variety of weird edge cases, all of which need to be carefully reasoned about. Fundamentally, fetch() gives a hosting app a privileged position, 'as if' they were a MITM. Now, CORS exists to prevent them from munging with third-party content, as do Opaque responses (whatever the implications are for HTTP in https://github.com/whatwg/fetch/issues/69 ), so the majority of the attack is that similar to a 'hostile ISP' or 'hostile WiFi' attack. However, that's still fundamentally an attack model to be considered.

For example, an attacker who can truncate headers - even without the ability to inspect headers - used to be able to do very naughty things. https://code.google.com/p/chromium/issues/detail?id=244260 unquestionably remains my favourite demonstration of this (before Antoine Delignat-Lavaud and Karthikeyan Bhargavan decided to go break everything else in TLS :+1: ). So that's one model - headers.

Once we get past headers, we have to consider body. For chunked encoding, could someone alter the contents or expectations if they could mount a truncation attack? Would ServiceWorkers / CancelablePromises amplify this attack from being "theoretically possible by a hostile wifi" to "easily executed by simply loading a page"?

I also discussed with Jake about some of the XHR behaviours, namely with respect to the cache (browser cache, not Cache object), and about wanting to make sure we don't cache content in a way that misrepresents what the site actually sends. One answer could be "Don't cache any incomplete response", but that might be limiting to some set of use cases (for example, could/should fetch() allow you to resume connections if you have a partial object in the disk-cache or Cache-object?)

I don't have answers, broadly speaking, nor do I have particular targeted attacks productionized that might be specifically mitigated. Instead, I wanted to raise the broader point, demonstrate a few examples of 'badness', and then see if there's a common pattern/feature we can extract and some basic mitigations we should deploy.

domenic commented 9 years ago

It's not clear to me what new concerns are present vs. xhr.abort()?

sleevi commented 9 years ago

@domenic I spelled out above just some of the ways in which it's different from xhr.abort(). Perhaps it requires a greater contextual awareness of how XHR is implemented in Chromium, but there are quite a few differences between the two.

These are two very different codepaths, so even if there is conceptual similarity, the attack surface and analysis is vastly different.

annevk commented 9 years ago

XMLHttpRequest cannot be used to feed arbitrary responses to any existing API, for one. fetch() combined with service workers, can.

annevk commented 9 years ago

I explicitly mentioned "no-cors" in OP, which is a feature XMLHttpRequest does not have, too.

domenic commented 8 years ago

Going to move this to the fetch repo... Going to edit the OP of https://github.com/whatwg/fetch/issues/27 to point here.