Closed Bug 1478843 (CVE-2018-12391) Opened 2 years ago Closed 2 years ago

Cross-origin audio leak in HLS

Categories

(Firefox for Android :: Audio/Video, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 64
Tracking Status
firefox-esr60 63+ fixed
firefox62 --- wontfix
firefox63 + verified
firefox64 + verified

People

(Reporter: s.h.h.n.j.k, Assigned: padenot)

References

Details

(Keywords: csectype-sop, regression, sec-high, Whiteboard: [adv-main63+][adv-esr60.3+])

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.75 Safari/537.36

Steps to reproduce:

1. Go to https://test.shhnjk.com/hls/webvideo.html
2. Wait for video to stop playing and observe the alert saying "super bad"


Actual results:

Cross-origin audio data leaks. Audio tag on the right side will play stolen sound.


Expected results:

HTTP Live Stream is supported in Firefox for Android. And the M3U8 file allows audio data to be fetched from other origin. But Firefox only checks the url and misses the following redirect in https://test.shhnjk.com/hls/same.m3u8

#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID="audio",NAME="English stereo",LANGUAGE="en",AUTOSELECT=YES,URI="https://test.shhnjk.com/location.php?url=https://vuln.shhnjk.com/audio.m3u8"

Where audio file is redirected and provided from "https://vuln.shhnjk.com/audio.m3u8". We can abuse this missed check to steal the audio samples using Web Audio API and read the audio data across-origin.
:drno, :padenot, seems :jya is on PTO so pinging you about this - feel free to redirect to someone else who knows our Fennec AV stack better.

Jun: thanks for the report. I'm assuming, based on you filing this against Firefox for Android, that this doesn't affect desktop (presumably because we're using platform support for m3u8 on Android, but don't do that elsewhere )?
Flags: needinfo?(s.h.h.n.j.k)
Flags: needinfo?(padenot)
Flags: needinfo?(drno)
I don't think Firefox supports HLS (.m3u8 files) in Desktop. If that's supported in any other platform, then it might be affected.
Flags: needinfo?(s.h.h.n.j.k)
Nick, you've reviewed some of this before. It looks like the right people are gone.

I can't repro on Nightly, but I can repro on release. On Nightly, the page does not seem to work, and Nightly eventually crashes [0].

Firefox Desktop does not support m3u8 playback so we're good there.

We think that we're handling cross-origin issues at [1]. Looks like we're doing some principal checking there but it's not about the resource, it's the URL of the manifest, so the CORS check passes.

[2] is probably where the problem lies: you can configure to bail out when a cross-protocol redirect occurs, but nothing about principals.

The root of the problem is that we're letting something that is not Gecko perform an HTTP connection to a URL that can be controlled by content, and that's VERY BAD. This bit of Java does not know anything about CORS, and is upstream code (that we appear to have patched). One solution is to plumb down a principal CORS to DefaultHttpDataSource.java and teach it about CORS, but maybe there is something better we can do.

I'm pretty sure I can trivially turn this into an arbitrary cross-origin resource access (text/image/whatever) with a variant of [3].

cc-ing network people because this is a bit bigger in scope that I would have anticipated.

[0]: https://crash-stats.mozilla.com/report/index/364db8b3-a426-4ef5-91f0-3cd8a0180727
[1]: https://searchfox.org/mozilla-central/source/dom/media/hls/HLSDecoder.cpp#146
[2]: https://searchfox.org/mozilla-central/source/mobile/android/geckoview/src/thirdparty/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java#360
[3]: https://jakearchibald.com/2018/i-discovered-a-browser-bug/
Flags: needinfo?(padenot)
Flags: needinfo?(nalexander)
Flags: needinfo?(jduell.mcbugs)
Yowza.  I have no idea about any of the details of exoplayer2 -- I really was involved only to get the build system plumbing arranged.  I'm not really surprised that Fennec's integration of that component caused issues in this manner:a mismatch of expectations between the browser and the consumed library.  Score another one for really owning our network stack, even in our component libraries.  My understanding is that this is _not_ an issue with exoplayer2, i.e., that this is an issue only because of the same-origin policy that the browser needs to enforce, which arbitrary exoplayer2 consumers don't care about.

I think the best thing I can do is expand the discussion to include snorp, who can a) vet my understanding of whether this needs to be upstreamed to exoplayer2, b) comment on whether this is relevant to GeckoView and/or future Android browsers, and c) redirect further to actually fix the underlying issue, or make the decision to disable exoplayer2/HLS in Firefox for Android.
Flags: needinfo?(nalexander) → needinfo?(snorp)
I got referred to ask dragana and valentin for advice on this one. Based on Paul's comment #3 can help us to assess the situation here?
Flags: needinfo?(valentin.gosu)
Flags: needinfo?(drno)
Flags: needinfo?(dd.mozilla)
Status: UNCONFIRMED → NEW
Rank: 7
Ever confirmed: true
Priority: -- → P1
If we can't change exoplayer2 do we know enough in the Gecko layers to simply taint all HLS video, even same-origin ones? If most sites that use it are just playing the content (and it's not supported on Desktop anyway) maybe that's good enough?

Do we use exoplayer2 for other things? If so then the lack of CORS support is going to bite lots of other places.
(In reply to Paul Adenot (:padenot) from comment #3)
> I'm pretty sure I can trivially turn this into an arbitrary cross-origin
> resource access (text/image/whatever) with a variant of [3].
> [3]: https://jakearchibald.com/2018/i-discovered-a-browser-bug/
I'm trying to see if it's possible, but crash is stopping me in stable build. Can anyone analyze the crash?

https://test.shhnjk.com/hls/try_fetch.html
Flags: sec-bounty?
I will ad Christoph as well.
Flags: needinfo?(dd.mozilla) → needinfo?(ckerschb)
Can we get info about redirects from exoplayer2?
(In reply to Dragana Damjanovic [:dragana] from comment #10)
> Can we get info about redirects from exoplayer2?

Unclear, I've just been reading the code for now. It does follow redirect [0], but maybe you have something else in mind?

[0]: https://searchfox.org/mozilla-central/source/mobile/android/geckoview/src/thirdparty/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java#355
(In reply to Paul Adenot (:padenot) from comment #11)
> (In reply to Dragana Damjanovic [:dragana] from comment #10)
> > Can we get info about redirects from exoplayer2?
> 
> Unclear, I've just been reading the code for now. It does follow redirect
> [0], but maybe you have something else in mind?
> 
> [0]:
> https://searchfox.org/mozilla-central/source/mobile/android/geckoview/src/
> thirdparty/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.
> java#355

exoplayer2 follows redirects I have seen that. Gecko does not know about these redirects. If gecko would know that a cross-origin redirect is made it could make proper decisions and avoid this problem. My question was is there a way the exoplayer2 tells gecko that it follow a redirect?
(In reply to Daniel Veditz [:dveditz] from comment #6)
> If we can't change exoplayer2 do we know enough in the Gecko layers to
> simply taint all HLS video, even same-origin ones? If most sites that use it
> are just playing the content (and it's not supported on Desktop anyway)
> maybe that's good enough?

Yes, attaching a patch that does that if that's what we want. The patch is very simple.

> Do we use exoplayer2 for other things? If so then the lack of CORS support
> is going to bite lots of other places.

It's only used to playback streams in HTMLMediaElement on Android (no other parts of Gecko make use of it). However it's plausible that some content is trying to do something more complex with those HTMLMediaElement (captureStream, Web Audio, canvas, etc.).

We could teach exoplayer2 about CORS, it does not seem too complicated from a quick look. We could also find a way to find the URL from which the media is currently being fetched from (after all redirects, etc.) and use Gecko facilities to determine the CORS status. I haven't seen an API call to do so (but haven't looked very hard), but it does not seem too crazy to add.

We could also wrap Gecko channnnels in a Java interface and inject it into exoplayer2. It's a bit weird to have something making http(s) requests randomly in Java code, in the middle of the media decoder of a browser engine.
(In reply to Dragana Damjanovic [:dragana] from comment #12)
> exoplayer2 follows redirects I have seen that. Gecko does not know about
> these redirects. If gecko would know that a cross-origin redirect is made it
> could make proper decisions and avoid this problem. My question was is there
> a way the exoplayer2 tells gecko that it follow a redirect?

Not in the current state of the code it looks like unfortunately.
This makes all HLS stream CORS-cross-origin.
Flags: needinfo?(valentin.gosu)
(In reply to Dragana Damjanovic [:dragana] from comment #12)
> exoplayer2 follows redirects I have seen that. Gecko does not know about
> these redirects. If gecko would know that a cross-origin redirect is made it
> could make proper decisions and avoid this problem. My question was is there
> a way the exoplayer2 tells gecko that it follow a redirect?

Dragana's suggestion here sounds like the best path forward to me, if technically feasible: Hooking up exoplayer2 redirects with Gecko internal redirects (e.g. by then calling nsContentSecuriyManager::AsyncOnChannelRedirect()) would automagically perform the right security checks.

Is it possible to get callbacks into Gecko whenever exoplayer2 follows a redirect?
Flags: needinfo?(ckerschb)
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #16)
> (In reply to Dragana Damjanovic [:dragana] from comment #12)
> > exoplayer2 follows redirects I have seen that. Gecko does not know about
> > these redirects. If gecko would know that a cross-origin redirect is made it
> > could make proper decisions and avoid this problem. My question was is there
> > a way the exoplayer2 tells gecko that it follow a redirect?
> 
> Dragana's suggestion here sounds like the best path forward to me, if
> technically feasible: Hooking up exoplayer2 redirects with Gecko internal
> redirects (e.g. by then calling
> nsContentSecuriyManager::AsyncOnChannelRedirect()) would automagically
> perform the right security checks.
> 
> Is it possible to get callbacks into Gecko whenever exoplayer2 follows a
> redirect?

Not using current APIs offered by exoplayer2 for now. It's doable though.
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #16)
> (In reply to Dragana Damjanovic [:dragana] from comment #12)
> > exoplayer2 follows redirects I have seen that. Gecko does not know about
> > these redirects. If gecko would know that a cross-origin redirect is made it
> > could make proper decisions and avoid this problem. My question was is there
> > a way the exoplayer2 tells gecko that it follow a redirect?
> 
> Dragana's suggestion here sounds like the best path forward to me, if
> technically feasible: Hooking up exoplayer2 redirects with Gecko internal
> redirects (e.g. by then calling
> nsContentSecuriyManager::AsyncOnChannelRedirect()) would automagically
> perform the right security checks.
> 
> Is it possible to get callbacks into Gecko whenever exoplayer2 follows a
> redirect?

I think this path is the best technically and produces the best outcome, but the cost is high and Fennec is in deep maintenance mode.  I would follow this approach only if the exoplayer2 stack will be used for GeckoView.  I do see that the thirdparty source is in geckoview/src/thirdparty/exoplayer2, so that seems positive.  Even if we do try to "do the right thing", I think we should reconsider the correct path for HLS support in GeckoView.  Is the best place to work...

- our own stack (expensive to build on Android, but the best outcome?)
- upstream exoplayer2 (a chance to update to newer exoplayer2 and solve this problem at the right level)
- our vendored exoplayer2 (little chance of breaking other things)

I'd like snorp to weigh in before we spend too much time trying to pursue the "correct" fix for exoplayer2.
(In reply to Nick Alexander :nalexander from comment #18)
> 
> - our own stack (expensive to build on Android, but the best outcome?)

IIRC, this wasn't feasible due to patent issues, but I also remember ajones telling me this gets better at some point. 2019?

> - upstream exoplayer2 (a chance to update to newer exoplayer2 and solve this
> problem at the right level)
> - our vendored exoplayer2 (little chance of breaking other things)
> 
> I'd like snorp to weigh in before we spend too much time trying to pursue
> the "correct" fix for exoplayer2.

I don't have a strong opinion on either of those options. Depending on the patent situation, we may not need to use exoplayer much longer. It's also not clear if HLS will remain relevant.
Flags: needinfo?(snorp)
Is this bug fixed per comment 15?
What do we want to do here? It seems we don't have resources to properly fix this at the minute, do we want to land my patch so that at least the hole is plugged?
(In reply to Paul Adenot (:padenot) from comment #21)
> What do we want to do here? It seems we don't have resources to properly fix
> this at the minute, do we want to land my patch so that at least the hole is
> plugged?

I would ask Christoph, but he is on PTO. Tanvi?
Flags: needinfo?(tanvi)
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #19)
> I don't have a strong opinion on either of those options. Depending on the
> patent situation, we may not need to use exoplayer much longer. It's also
> not clear if HLS will remain relevant.

AFAIK, almost any streams on android is an HLS one... so it's still very relevant on mobile...
(In reply to Paul Adenot (:padenot) from comment #15)
> Created attachment 8995893 [details] [diff] [review]
> hls-cors-cross-origin.patch
> 
> This makes all HLS stream CORS-cross-origin.

Can you describe in more detail the consequence of this change?
(In reply to Tanvi Vyas[:tanvi] from comment #24)
> (In reply to Paul Adenot (:padenot) from comment #15)
> > Created attachment 8995893 [details] [diff] [review]
> > hls-cors-cross-origin.patch
> > 
> > This makes all HLS stream CORS-cross-origin.
> 
> Can you describe in more detail the consequence of this change?

The change treats all HLS stream as cross-origin, even for same-origin resources.
See Also: → 1486335
Flags: needinfo?(dveditz)
Duplicate of this bug: 1486335
I'm planning to present this bug on December too (though I'm not sure if I'll be selected). I can remove Firefox content if this won't be fix till December, but other browser has similar issue, so I expect other people can discover this bug too.
(In reply to Paul Adenot (:padenot) from comment #21)
> What do we want to do here? It seems we don't have resources to properly fix this
> at the minute, do we want to land my patch so that at least the hole is plugged?

Yes. Treating all HLS streams as opaque is perfectly fine.
Flags: needinfo?(tanvi)
Flags: needinfo?(dveditz)
Could anyone please land patch in comment 15 to at least close the hole?
Attachment #8995893 - Flags: review?(jyavenard)
Comment on attachment 8995893 [details] [diff] [review]
hls-cors-cross-origin.patch

Review of attachment 8995893 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/hls/HLSDecoder.cpp
@@ +182,5 @@
>  already_AddRefed<nsIPrincipal>
>  HLSDecoder::GetCurrentPrincipal()
>  {
>    MOZ_ASSERT(NS_IsMainThread());
> +  return nullptr;

This needs comments.
Why it's done that way and the downside in doing so.
Comment on attachment 8995893 [details] [diff] [review]
hls-cors-cross-origin.patch

Review of attachment 8995893 [details] [diff] [review]:
-----------------------------------------------------------------

I understand the need to be discrete here on the vulnerability, but I'm concerned that the method as it is now will have its reasoning disappear into oblivion in a short amount of time.

So some explicit reference to this bug number at least is I believe necessary
Attachment #8995893 - Flags: review?(jyavenard) → review+
Assignee: nobody → padenot
(In reply to Paul Adenot (:padenot) from comment #32)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/
> 19d5e50db843b03d4aed8e5f5736a531035c9cf0 (added a comment).

Uh, shouldn't this have had sec-approval to land, given it's sec-high? What are we doing about branches?
Flags: needinfo?(padenot)
There was private irc conversation with the security team, asking me to land this. All branches are affected, but we'll let this sit a bit on central before requesting uplift.
Flags: needinfo?(padenot)
https://hg.mozilla.org/mozilla-central/rev/19d5e50db843
Group: firefox-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
(In reply to Paul Adenot (:padenot) from comment #34)
> There was private irc conversation with the security team, asking me to land
> this. All branches are affected, but we'll let this sit a bit on central
> before requesting uplift.

For posterity's sake, I talked to padenot and asked him if this bug could move forward.
I didn't clearly explain that he needs separate(!) sec-approval on the patch, that I'm not entitled to grant.
I should have made clear during the conversation. 

On a more uplifting note, padenot also tells me that there are other, unrelated bugs that actually stop the exploit from working.
Please nominate this for Beta and ESR60 approval when you get a chance.
Flags: needinfo?(padenot)
Comment on attachment 8995893 [details] [diff] [review]
hls-cors-cross-origin.patch

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1345752

User impact if declined: Possible vector for CORS leak, however other bugs seem to prevent this being exploitable.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Medium

Why is the change risky/not risky? (and alternatives if risky): Some web-compat risk, we don't really know how many HLS streams are on other origin (probably quite a few), but the current code is  completely incorrect and dangerous, so we're disabling it here.

String changes made/needed: None
Flags: needinfo?(padenot)
Attachment #8995893 - Flags: approval-mozilla-beta?
Comment on attachment 8995893 [details] [diff] [review]
hls-cors-cross-origin.patch

Uplift approved for 63 beta 13, thanks.
Attachment #8995893 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
RyanVM: Do we even have a Fennec ESR? As this is android-only we shouldn't need an ESR uplift.
Flags: sec-bounty?
Flags: sec-bounty+
Flags: needinfo?(ryanvm)
Flags: needinfo?(jduell.mcbugs)
Keywords: regression
Touche.
Flags: needinfo?(ryanvm)
Flags: needinfo?(sorina.florean)
I think this bug needs follow up (e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=1486335)
I was able to reproduce this issue following the steps from the description on Release 62.0.3 and I can confirm that this is fixed on 63.0b13 and latest Nightly build 64.0a1 with Motorola Nexus 6(Android 7.1.1) and Sony Xperia Z5 Premium(Android 6.0.1).
Status: RESOLVED → VERIFIED
Flags: needinfo?(sorina.florean)
Per discussion on IRC, we're going to take this on ESR after all to benefit the Tor folks (who ship an Android browser from that branch).
Attachment #8995893 - Flags: approval-mozilla-esr60+
Whiteboard: [adv-main63+][adv-esr60.3+]
Alias: CVE-2018-12391
See Also: → 1505308
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.