1162411 - Cross origin restriction bypass with Fetch API

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Muneaki Nishimura]

Attachment: Capture.PNG

User Agent: Mozilla/5.0 (Windows NT 6.2; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2383.0 Safari/537.36

Steps to reproduce:

1) Open Fiddler web debugger and enable "Decrypt HTTPS Traffic" option
2) Launch https://mallory.csrf.jp/fetch/request.html
3) See the request packet sent from the 2) by Fiddler


Actual results:

Fetch API doesn't restrict request header names to be set.
In this scenario, the request shown in 3) has invalid Host and Cookie headers to the other origin, csrf.jp, and the response data has come from https://csrf.jp/index.html.

fetch("/index.html", {
  method: "GET",
  headers: {
    "Host": 'csrf.jp',
    "Cookie": "Arbitorary Values"
  }
})


Expected results:

Fetch API should restrict header names that can be set (see the attached image).
For example, Host, Cookie and sec-* headers should be ignored.
The implementation of XmlHttpRequest.setRequestHeader would be a good reference.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Per spec https://fetch.spec.whatwg.org/#fetch-method calls https://fetch.spec.whatwg.org/#dom-request which in this case will end up in step 26 calling https://fetch.spec.whatwg.org/#concept-headers-fill which calls <https://fetch.spec.whatwg.org/#concept-headers-append>.  The guard is "request", since that's what https://fetch.spec.whatwg.org/#dom-request set up, so things that are on the "forbidden header name" list should be ignored.  That list is at https://fetch.spec.whatwg.org/#forbidden-header-name and includes "Host".  And "Cookie".

That said, we seem to implement at least parts of that... Need to figure out what's going on here.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Ah, looks like the bug is that Request::Constructor never sets the guard on the Headers object to "request" anywhere.

Comment 3

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: fix

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8603047 [details] [diff] [review]
fix

r=me

Comment 5

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8635863a78c0

Comment 6

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

[Tracking Requested - why for this release]:
Security bug that allows cross origin restriction bypass in the new fetch api.

Comment 7

[Andrew Overholt [:overholt]]

Is it possible to get this into 38 or a 38 point release?  Given it's the next ESR should it just wait until 38's first point release?

Comment 8

[Boris Zbarsky [:bzbarsky]]

The Fetch API is not enabled by default in 38.  We only enabled it in 39 (see bug 1133861).

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

Given that this is a sec bug, it should have a sec rating and may have required sec approval to land. From comment 8 I take it that this fix isn't required for ESR but should be uplifted to 39, which becomes Beta on Monday.

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/a8521f7c90a2 for mochitest-1 orange:

https://treeherder.mozilla.org/logviewer.html#?job_id=9691431&repo=mozilla-inbound

Comment 11

[Al Billings [:abillings - ex-MoCo]]

Yes, this needs a rating and sec-approval+ before any checkin.

https://wiki.mozilla.org/Security/Bug_Approval_Process

Comment 12

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8603047 [details] [diff] [review]
fix

This is my first time, so I hope I got things right.
Should I publicly be pushing to inbound?

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
The test case is a potential exploit

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Yes

Which older supported branches are affected by this flaw?
The feature is not in release builds yet. It was shipped in Fx 39 and is in Dev edition.

If not all supported branches, which bug introduced the flaw?
Bug 1039846

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
The patch should apply directly. Otherwise, it is easy to create a backport.

How likely is this patch to cause regressions; how much testing does it need?
Low. Tests already exist and this patch adds a few more.

Comment 13

[Al Billings [:abillings - ex-MoCo]]

We need a security rating on this. Could you take a look at https://wiki.mozilla.org/Security_Severity_Ratings

FYI, you won't be able to check in for a few weeks. We're shipping on Tuesday next week and starting a new six week dev cycle and, normally, we don't allow security bugs to be checked in during the first two weeks unless there is a very good reason.

Comment 14

[Boris Zbarsky [:bzbarsky]]

It's already been checked in (and then backed out), so I'm not sure how much of a point there is to delaying it landing at this point, honestly.

Comment 15

[Al Billings [:abillings - ex-MoCo]]

Well, it still needs a rating. Otherwise, we can't write advisories (among other things).

Comment 16

[Boris Zbarsky [:bzbarsky]]

I'm not disagreeing about that part.  This is probably a sec-high: it's a cross-origin policy violation allowing reading data cross-site (though not from arbitrary sites, only ones on the same IP), but doesn't allow privilege escalation to arbitrary execution or anything like that.  I don't think the same-IP restriction is enough to downgrade to a sec-moderate, personally.

Comment 17

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8603047 [details] [diff] [review]
fix

Thanks for the rating. I'll mark this as sec-approval+. Might as well check it in given the previous exposure.

Comment 18

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4d40d05a58df

Comment 19

[(no longer active)]

Muneaki, Chrome seems to be similarly affected.  Have you reported this to the Chromium project?  Thanks!

Comment 20

[Muneaki Nishimura]

Ehsan, I've checked it on the latest Chrome (44.0.2383.0 m) but as far as I looked illegal headers  were implicitly removed from HTTP requests. If you found some ways to bypass the restriction on Chrome please report it from you.

And note that I also reported another cross origin violation issue of Fetch API (with ServiceWorker) as Bug 1162018. Please check the bug if you've never seen.

Comment 21

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/4d40d05a58df

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

Nikhil can you request uplift to beta since it looks like 39 is affected? Thanks.

Comment 23

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8603047 [details] [diff] [review]
fix

Approval Request Comment
[Feature/regressing bug #]: Bug 1039846
[User impact if declined]: Websites can spoof headers like User-Agent and Origin, as well as pass cookies.
[Describe test coverage new/current, TreeHerder]: Patch has tests.
[Risks and why]: None. This API only shipped in Firefox 39.
[String/UUID change made/needed]: None.

Comment 24

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8603047 [details] [diff] [review]
fix

Approved for uplift to beta (39) since this is a sec-high bug.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

The test changes need rebasing around bug 1149987.

Comment 26

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: fix for beta

This is ready to go.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/90fba299cd36

Comment 28

[Andrei Vaida [:avaida]]

Reproduced the initial issue on Aurora 39.0a2 (2015-05-05), using the steps and sample page from Comment 0.

Per my conversation with :nsm, this is now verified fixed on 39.0b1-build1 (20150520170903) - the request is no longer setting a cookie.