Closed
Bug 1441153
Opened 7 years ago
Closed 7 years ago
Cross origin leak of resource size using media element
Categories
(Core :: Audio/Video: Playback, defect, P2)
Tracking
()
VERIFIED
FIXED
mozilla60
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox58 | --- | unaffected |
firefox59 | + | fixed |
firefox60 | + | fixed |
People
(Reporter: jaffathecake, Assigned: cpearce)
References
()
Details
(Keywords: csectype-sop, regression, sec-moderate)
Attachments
(1 file)
3.01 KB,
patch
|
jya
:
review+
lizzard
:
approval-mozilla-release+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3352.0 Safari/537.36 Steps to reproduce: https://jewel-chair.glitch.me/exploit.html?url=https://annevankesteren.nl/ I'm able to determine the content length of https://annevankesteren.nl/ when requested with credentials. This affects any server that supports partial responses. Actual results: My server returns a valid PCM wav header for a 44100hz 8bit mono wav and uses a Content-Range header to suggest there's more to come. Firefox then makes a request for further data. However, I respond with a redirect to a cross origin URL. Firefox follows the redirect, and requests the cross-origin site for a particular range. The cross-origin server responds, and Firefox sends the data into the audio element. Since PCM is raw, the amount of data received is reflected in the duration of the media element. Expected results: Chrome doesn't follow the redirect, which seems reasonable.
Reporter | ||
Comment 1•7 years ago
|
||
Although I can only see the content length here, I'm worried there may be ways of exposing particular bytes by tricking Firefox into treating cross-origin content as valid media metadata.
Comment 2•7 years ago
|
||
:padenot/:achronop can you (find someone more suitable) to take a look? Thanks!
Group: firefox-core-security → core-security
Component: Untriaged → Audio/Video
Flags: needinfo?(padenot)
Flags: needinfo?(achronop)
Product: Firefox → Core
Comment 3•7 years ago
|
||
https://github.com/whatwg/fetch/issues/145 has more details on Chrome's implementation. roc suggested in a comment there we were not affected, but maybe he did not consider all angles.
Comment 4•7 years ago
|
||
The resource is cross origin. Direct playback works well (it however play garbage, and this is correct behaviour). Trying to inspect the bytes of the resource however errors out as you can imagine: > var ac = new AudioContext() > ac.createMediaElementSource(audio) The HTMLMediaElement passed to createMediaElementSource has a cross-origin resource, the node will output silence. There are various ways to try to get the content of the resource, but they all involve getting a `MediaStream` or piping the `HTMLMediaElement` to an `AudioContext`, and this is handled correctly. Using the metadata is a good idea but has very strong constraint: we can only _indirectly_ observe the following characteristics (because they have an influence on the duration, open http://soundfile.sapp.org/doc/WaveFormat/ to follow along): - the audio format - the sample-rate - the channel count - the bit rate but they have to be followed by valid values for the alignment and the bit per sample fields. Also, those values are rather strictly checked by Firefox, and anything more than (saying this off the top of my head) 8 channels or 192kHz, or a weird format / bit-rate will error out. It does not seem possible to reliably read bytes this way. Maybe with other format this are a bit easier, but I'd doubt it. In any case, it seems that the length of the resource is indeed leaked. Granted Chrome does not allow playback of media data after a redirect + origin change, if `crossOrigin` has not been set on the HTMLMediaElement, it's probably not necessary to be able to do it in Firefox as well. Chris, do you agree?
Flags: needinfo?(padenot)
Flags: needinfo?(cpearce)
Flags: needinfo?(achronop)
Reporter | ||
Comment 5•7 years ago
|
||
Yeah, I can see that getting bytes via wav isn't possible, but I'm worried about other container formats. Flac's streaminfo block ends with a sample count, followed by MD5. It'd error quickly… it depends on whether the audio element will expose duration before the error.
Assignee | ||
Comment 6•7 years ago
|
||
I agree, we shouldn't be leaking the content-length. We shouldn't try to play resources after a cross-origin redirect if crossOrigin is set. In 58.0.2 in Ubuntu's Firefox release build and in Mozilla-built release Firefox 58.0 on MacOS I don't see the information leak, but I do see it in Nightly on MacOS. This must be a recent regression. I ran a moz-regression, and found this was a regression from https://hg.mozilla.org/integration/autoland/rev/c5e45c611460 in bug 1418430. We'd better either back out bug 1418430, or fix the bug.
Assignee: nobody → cpearce
Flags: needinfo?(cpearce)
Assignee | ||
Comment 7•7 years ago
|
||
We should track this bug and consider uplifting a fix to 59, as it's a cross origin information leak.
status-firefox58:
--- → unaffected
status-firefox59:
--- → affected
status-firefox60:
--- → affected
tracking-firefox59:
--- → ?
Component: Audio/Video → Audio/Video: Playback
Version: Trunk → 59 Branch
Comment 8•7 years ago
|
||
> We shouldn't try to play resources after a cross-origin redirect if crossOrigin is set. For CORS requests it should be fine, as long as every response, including the redirects, opts into CORS. I sure hope we enforce that already, otherwise we have bigger problems. The attack is about requests that don't use CORS. https://fetch.spec.whatwg.org/ calls this "no-cors". And as per the issue I linked to we cannot simply forbid redirects. Mike West pointed out that would break deployed content. Instead we have to require all content to come from the same (final) origin, or some such.
Assignee | ||
Comment 9•7 years ago
|
||
The HTTP headers from the response are: $ curl -I https://jewel-chair.glitch.me/audio-exploit?url=https://annevankesteren.nl/ HTTP/1.1 206 Partial Content Date: Wed, 28 Feb 2018 00:09:28 GMT Content-Type: audio/wav Content-Length: 44 Connection: keep-alive x-powered-by: Express cache-control: no-cache accept-ranges: bytes content-range: bytes 0-43/79380044 etag: W/"2c-mpzw6LEMTJO6EJo3dllXg4EA9ps" Prior to Bug 1418430 we'd assume the stream ended at 44 bytes, and not re try when the connection redirects. After Bug 1418430 we retry, as we now pass aReopenOnError=true to MediaCacheStream::NotifyDataEnded() in ChannelMediaResource::OnStopRequest(). AFAICT, we are still doing what roc says here: https://github.com/whatwg/fetch/issues/145#issuecomment-152432861 That is, we follow redirects from the initial load in order to find the principal of the load, and then during streaming we follow subsequent redirects but whether they're cross origin, and if they're not CORS allowed we still play the media, but we try to restrict information leakage by disabling some APIs. This is why Jake pointed that you can't access media data after the redirect. The problem is that since the WAV format is uncompressed, you can infer the length of the resource from the duration. So we need to not leak the duration somehow in cross origin requests which aren't allowed via CORS. So we could either error when a WAV resource redirects to CORS non-same-origin (i.e. cross origin and no Allow-Origin headers and CORS mode) resources, or we could report the duration and currentTime as NaN. I can't think of any other fixed size formats that we support.
Updated•7 years ago
|
Blocks: 1418430
Group: core-security → media-core-security
Status: UNCONFIRMED → NEW
status-firefox-esr52:
--- → unaffected
Ever confirmed: true
Summary: Cross origin data leak using media element → Cross origin leak of resource size using media element
Reporter | ||
Comment 10•7 years ago
|
||
I'm working on defining how a service worker can sit in the middle of this. My plan is to fail at the media element level. If, for a given piece of media, it has processed opaque data, and the places it has received content from (after redirects) differ, error. Trying to patch this just for wav seems risky. We don't know what might happen with future formats, or if there's a way to achieve the same with other formats, eg oggpcm.
Assignee | ||
Comment 11•7 years ago
|
||
Assignee | ||
Comment 12•7 years ago
|
||
(In reply to Jake Archibald from comment #10) > Trying to patch this just for wav seems risky. We don't know what might > happen with future formats, or if there's a way to achieve the same with > other formats, eg oggpcm. While certainly I appreciate your argument, I don't think we've got a better solution. I don't think we can just start blocking mid-flight cross-origin redirects. We'll break things. I think it's unlikely we'll be adding any more uncompressed formats to the web platform. The trend is towards better compression of formats, not less.
Assignee | ||
Comment 13•7 years ago
|
||
Comment on attachment 8954994 [details] [diff] [review] Block cross origin redirects after media channel setup for wav Jean-Yves: Can you please review this patch? Here we have a web server that responds to Firefox's initial request with a 44 byte long 206 byte range response of a WAV header. When Firefox does a second HTTP byte range request for the rest of the media, the server responds with a cross origin redirect. If Firefox follows the redirect and loads the rest of the resource, because WAV is an uncompressed format, then the WAV's duration visible in HTMLMediaElement.duration is a function of the Content-Length of the (redirected to) resource. This means an attacker can use this technique to sniff out resources they're not supposed to be able to see, which is a security problem. The solution here is to block mid-flight cross origin redirects for WAV. I don't think we have any other fixed size formats that we need to worry about.
Attachment #8954994 -
Flags: review?(jyavenard)
Reporter | ||
Comment 14•7 years ago
|
||
(In reply to Chris Pearce (:cpearce) from comment #12) > While certainly I appreciate your argument, I don't think we've got a better > solution. I don't think we can just start blocking mid-flight cross-origin > redirects. We'll break things. But this is what Chrome already does.
Comment 15•7 years ago
|
||
Comment on attachment 8954994 [details] [diff] [review] Block cross origin redirects after media channel setup for wav Review of attachment 8954994 [details] [diff] [review]: ----------------------------------------------------------------- What about mp3? that too attempt to determine the duration from the content length... though it needs to have a valid start
Attachment #8954994 -
Flags: review?(jyavenard) → review+
Reporter | ||
Comment 16•7 years ago
|
||
For reference, here's some info on Chrome's behaviour https://github.com/whatwg/fetch/issues/145.
Assignee | ||
Comment 17•7 years ago
|
||
(In reply to Jake Archibald from comment #16) > For reference, here's some info on Chrome's behaviour > https://github.com/whatwg/fetch/issues/145. Key information about Chrome's behaviour is here: https://bugs.chromium.org/p/chromium/issues/detail?id=532569 Since M45 Chrome blocked mid-flight cross origin redirects, and in M47 they changed to allow mid-flight cross-origin redirects if they pass a CORS check. We don't really have the bandwidth or desire to make a behaviour change here, and manage any regressions here, especially so late in the beta cycle. It would be interesting to collect telemetry to see how often we encounter cross-origin redirects in the wild.
Assignee | ||
Comment 18•7 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b9dc9fb56ff89a43d79c0e604fac9be30ef2316
Comment 19•7 years ago
|
||
If this lands today and someone can verify the fix, it can still make the 59 RC build next Monday.
tracking-firefox60:
--- → +
Assignee | ||
Comment 20•7 years ago
|
||
(In reply to Chris Pearce (:cpearce) from comment #18) > Try: > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=0b9dc9fb56ff89a43d79c0e604fac9be30ef2316 test_seek_promise_bug1344357.html fails because it loads a WAV cross origin without CORS. Trying again with that test loading same origin: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2270f14d642b58765f4ded7f9f78ff59369de35f
Assignee | ||
Comment 21•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ac015e407553baf5f4e60fe1e021d8d3f884e1a
Assignee | ||
Comment 22•7 years ago
|
||
Backed out due to WP failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/4000e7883aefcc7886a7eabb43ac6b9e592a0066 Such as: https://treeherder.allizom.org/logviewer.html#?job_id=158047594&repo=mozilla-inbound&lineNumber=3299 3299 01:43:47 INFO - TEST-UNEXPECTED-TIMEOUT | /mixed-content/audio-tag/no-opt-in/cross-origin-http/top-level/keep-scheme-redirect/optionally-blockable/no-opt-in-allows.https.html | opt_in_method: no-opt-in 3306 01:43:47 INFO - TEST-UNEXPECTED-TIMEOUT | /mixed-content/audio-tag/no-opt-in/cross-origin-http/top-level/keep-scheme-redirect/optionally-blockable/no-opt-in-allows.https.html | expected OK
Assignee | ||
Comment 23•7 years ago
|
||
So what I failed to grok with the test_seek_promise_bug1344357.html failure is that we receive a ChannelMediaDecoder::NotifyPrincipalChanged() callback once all the initial channel's redirects are resolved. So we need to not bail in that first callback, only in subsequent calls to the callback. I bet that's the WP test failure too.
Assignee | ||
Comment 24•7 years ago
|
||
Try again: https://treeherder.mozilla.org/#/jobs?repo=try&revision=426be43cbda6a5c50f80151fc269eb075bafccd8&selectedJob=165437916
Assignee | ||
Comment 25•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1c354bfdde04756d61c8a4cf4d2769386ca1e224
Updated•7 years ago
|
Priority: -- → P2
Comment 26•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1c354bfdde04
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Comment 27•7 years ago
|
||
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #19) > If this lands today and someone can verify the fix, it can still make the 59 > RC build next Monday. This needs an approval request ASAP if that's the case.
Flags: needinfo?(cpearce)
Assignee | ||
Comment 29•7 years ago
|
||
Comment on attachment 8954994 [details] [diff] [review] Block cross origin redirects after media channel setup for wav Approval Request Comment [Feature/Bug causing the regression]: Bug 1418430 [User impact if declined]: A malicious site could determine whether cross origin exists and its file length by loading a WAV file from their server that redirects to a cross origin resource in the middle of the download. [Is this code covered by automated tests?]: The code changed is exercixed by existing tests, but we don't have a specific test for this yet. [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: It only changes our behaviour on WAV files that we're trying to play inside <audio> that contain HTTP redirects in the middle of the download. [String changes made/needed]: None
Flags: needinfo?(cpearce)
Attachment #8954994 -
Flags: approval-mozilla-beta?
Updated•7 years ago
|
Attachment #8954994 -
Flags: approval-mozilla-beta? → approval-mozilla-release?
Reporter | ||
Comment 30•7 years ago
|
||
Really appreciate everyone acting so quickly on this.
Comment 31•7 years ago
|
||
cpearce by "Verified" we usually mean "verified by someone who isn't the developer who worked on the patch"
Comment 32•7 years ago
|
||
Comment on attachment 8954994 [details] [diff] [review] Block cross origin redirects after media channel setup for wav Let's uplift to m-r for 59, even if this issue doesn't have a sec-high or sec-critical rating, it's a new regression in 59 so I'd like to see it fixed.
Attachment #8954994 -
Flags: approval-mozilla-release? → approval-mozilla-release+
Comment 33•7 years ago
|
||
uplift |
https://hg.mozilla.org/releases/mozilla-release/rev/1d5d7de28cef5e8bd6dca73360352caaf50ae826 (FIREFOX_59b_RELBRANCH) https://hg.mozilla.org/releases/mozilla-beta/rev/01ca4a01f130c63dd169cf1a58b66fe1b7881d2d
Assignee | ||
Comment 34•7 years ago
|
||
I had lunch with Roc the other day and discussed this. He convinced me we should block mid-flight redirects for all content types. The attack in particular we're worried about is an HTTP server serving a media header (of a non-WAV type) up to the duration (or something else observable through exposed metadata), and then serving the redirect, and then reading the duration/exposed metadata. While I think it's unlikely that our media parsers would parse the response after the redirect as valid media data, so it's unlikely any bytes would be readable, we should block this just in case. I think it's safe for that to ride the trains.
Assignee | ||
Comment 35•7 years ago
|
||
(In reply to Jake Archibald from comment #30) > Really appreciate everyone acting so quickly on this. Thanks for reporting the bug Jake, much appreciated!
Updated•7 years ago
|
Group: media-core-security → core-security-release
Updated•6 years ago
|
Updated•6 years ago
|
Group: core-security-release
Updated•6 years ago
|
Updated•6 years ago
|
Updated•3 years ago
|
See Also: → CVE-2022-31736
You need to log in
before you can comment on or make changes to this bug.
Description
•