Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Potential security issues when applied to fetch() #4

Closed
annevk opened this issue Jul 3, 2015 · 6 comments
Closed

Potential security issues when applied to fetch() #4

annevk opened this issue Jul 3, 2015 · 6 comments

Comments

@annevk
Copy link
Member

annevk commented Jul 3, 2015

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
Copy link

sleevi commented Jul 3, 2015

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 whatwg/fetch#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 👍 ). 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
Copy link
Member

domenic commented Jul 6, 2015

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

@sleevi
Copy link

sleevi commented Jul 7, 2015

@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
Copy link
Member Author

annevk commented Jul 7, 2015

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

@annevk
Copy link
Member Author

annevk commented Jul 7, 2015

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

@domenic
Copy link
Member

domenic commented Aug 22, 2016

Going to move this to the fetch repo... Going to edit the OP of whatwg/fetch#27 to point here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants