1738426 - (CVE-2023-5722) Allow opaque 206 responses into the cache

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect, P2)

Description

[Anne (:annevk)]

Ben Kelly from Google reached out to us about this.

This problem relates to how we handle range requests in cache_storage. Today we reject whenever you attempt to store a 206 in cache_storage. See step 6 here:

https://w3c.github.io/ServiceWorker/#cache-put

And step 4.7 here:
https://w3c.github.io/ServiceWorker/#cache-addAll

Confusingly the put language uses the inner response, but that is not done in the addAll language. In implementation, however, we do use the inner response in all cases. (I believe the spec intended to use inner response in all cases as well.)

This unfortunately allows one to infer some information about an opaque response being stored in cache_storage. If code makes a no-cors range request and then tries to put it in cache storage, if the put rejects then the code can infer that the opaque response is a 206.

This oracle then allows an attacker to iteratively probe the range of a server provided response. They simply increase their range size bit by bit until they discover that the server returns something other than a 206. This allows them to detect the size of an opaque response.

Unfortunately it appears servers handle out-of-range requests differently. Some return 416, but others return 200. I think this means we cannot simply also forbid 416 responses.

I think the only solution we have here is to not block 206 opaque responses. I think this is probably ok as simply an extension of the "you can't tell if this is a 200 or 404 anyway" footgun already present in opaque responses.

In the email thread I observed that there's also a problem with the Vary header. It seems attackers can learn its contents. If we cannot address that here a follow-up bug needs to be filed.

There's also the more general problem of the Cache API being broken around opaque responses. I'm not sure if there are other problems hiding there. It would be good to double check that.

Comment 1

[Daniel Veditz [:dveditz]]

Do you actually learn the content of the Vary header, or do you just have to brute-force everything and see what doesn't get cached?

Comment 2

[Anne (:annevk)]

You can brute-force the Cache API matching algorithm by supplying different requests. In Firefox you can also tell if Vary contains * as we reject a Cache API add in that case. (The specification does as well by operating on the internal response rather than the opaque response.) Reportedly Chrome does not. The Vary attacks are limited and a subset of the larger problem of the Cache API not treating opaque responses as opaque.

Comment 3

[Yaron Tausky [:ytausky]]

Are we checking the Vary header? From a cursory look here it seems like we only check the status code.

Comment 4

[Anne (:annevk)]

Ben pointed us toward https://searchfox.org/mozilla-central/rev/daa5c195e5efe45a63bf79f97ae77419b16f62f1/dom/cache/TypeUtils.cpp#188-193.

Comment 5

[Anne (:annevk)]

Chrome ended up fixing the Vary leaks too.

To be clear, I'd consider detecting the size of an opaque response higher than S3 as this can end up leaking user state.

Comment 6

[Jens Stutte [:jstutte]]

:asuth, any plan to get to this?

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Jens Stutte [:jstutte] from comment #6)

:asuth, any plan to get to this?

Good point, I won't be able to realistically get to this anytime soon with the priorities on my plate.

Comment 8

[Jens Stutte [:jstutte]]

We should find an assignee for this, though. Eden, would you mind to take a look?

Comment 9

[Eden Chuang[:edenchuang]]

Attachment: Bug 1738426 - Ignoring status 206 and vary header checking for opaque response in Cache API. r=asuth

Comment 10

[Andrew Sutherland [:asuth] (he/him)]

As a clarification I'm including in the review, it appears Firefox never had a problem with storing opaque 206 responses; we've always been checking the filtered status since our initial 206-blocking code landed in bug 1264181. This is consistent with the initial landing of the test changes Blink introduced when they addressed this bug on their end in https://chromium-review.googlesource.com/c/chromium/src/+/3251749 which was synced to WPT in https://github.com/web-platform-tests/wpt/pull/31421 and synced to m-c in bug 1738351. In particular, we can see that with the changes made to the test to pass when the opaque 206 is stored that we started passing the test. This behavior was maintained through the unification of Cache API tests to use ".any.js" instead of manually permuted tests in WPT change https://github.com/web-platform-tests/wpt/pull/36015 synced to m-c in bug 1792004 where we can see we still only failed the vary: * case. Note that this landing changed the paths in use, so following the history is not entirely trivial.

This means we only need the vary: * fix.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc0fac76d632
Ignoring status 206 and vary header checking for opaque response in Cache API. r=asuth

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dc0fac76d632

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:edenchuang, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox118 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Given the age of the bug, probably not something we need to uplift to 118 this late in the cycle. Leaving this on the radar for a possible ESR uplift request later, however.

Comment 15

[Tyson Smith [:tsmith]]

Attachment: advisory.txt

Comment 16

[Daniel Veditz [:dveditz]]

Bulk-unhiding security bugs fixed in Firefox 119-121 (Fall 2023). Use "moo-doctrine-subsidy" to filter