1188240 - Stagefright decoding used pre Android 4.1

Status: RESOLVED FIXED

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

Description

[Kevin Brosnan [Ex-Mozilla]]

It has been shown that specially encoded video and maybe audio can be used to exploit the Android OS. Firefox for Android running on 2.3, 3.x and 4.0 likely can be used to exploit this flaw. Not sure what we can do to safegaurd our users?

It would be useful if we can get our hands on a testcase.

The bug that implemented Android Media Codec decoding is https://bugzilla.mozilla.org/show_bug.cgi?id=1014614

Comment 1

[Christoph Diehl [:posidron]]

We

Comment 2

[Christoph Diehl [:posidron]]

We have a fuzzing meta bug here: https://bugzilla.mozilla.org/show_bug.cgi?id=fuzzing-stagefright
None of the found bugs got ever patched since 2013.

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

AFAICT, the actual vulnerabilities have not been made public yet.

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Some folks on HN seem to have figured things out based on fixes to CyanogenMod: https://news.ycombinator.com/item?id=9955431

It looks like they're all in the 3gpp and mp4 parser, neither of which we should be using with the new stuff (4.1+). We only use the decoder. Anthony, what do you think?

Comment 5

[Daniel Veditz [:dveditz]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #3)
> AFAICT, the actual vulnerabilities have not been made public yet.

Does it matter? If they're in the android system library, and it's un-patched, and we use that for video, we're probably hitting the bugs.

But we do know at least some of them since jduck reported one to us (fixed in Firefox 38), and said he had others that got fixed before he could report them (because other people reporterd them to us).

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Daniel Veditz [:dveditz] from comment #5)
> (In reply to James Willcox (:snorp) ([e-mail]) from comment #3)
> > AFAICT, the actual vulnerabilities have not been made public yet.
> 
> Does it matter? If they're in the android system library, and it's
> un-patched, and we use that for video, we're probably hitting the bugs.
> 

It does matter, because otherwise we won't know if we use that code or not. For instance, some of the vulnerabilities seem to revolve around 3gpp, which we don't (I think) support. Also, we could potentially detect malicious videos and refuse to play them.

Comment 7

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

We have seen a number of bugs relating to MP4. My suggestion would be to:

(a) Reject fragmented MP4 playback outright on pre-4.1 Android
(b) Dig out the 'moov' box and run it through our in-tree MP4 parser

The reason for (a) is that it isn't fMP4 isn't common enough outside of MSE to matter and it uses a separate code path. I don't know if we support 3gpp but we should just drop support for it.

Jean-Yves - does this seem like a sensible plan?

James/Ralph - can you two work together on this?

Comment 8

[Ralph Giles (:rillian)]

Are we talking about fennec, firefox os, or both?

Comment 9

[Kevin Brosnan [Ex-Mozilla]]

This specific bug is about Firefox for Android. Though if Firefox OS uses stagefright it should look into the patches from AOSP [1] but that is a different bug.

[1] https://news.ycombinator.com/item?id=9955431

Comment 10

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to Kevin Brosnan [:kbrosnan] from comment #9)
> This specific bug is about Firefox for Android. Though if Firefox OS uses
> stagefright it should look into the patches from AOSP [1] but that is a
> different bug.

This bug does not affect the latest Firefox OS.

Comment 11

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

(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #10)

> This bug does not affect the latest Firefox OS.

Which versions does it affect?

Comment 12

[Matt Wobensmith [:mwobensmith][:matt:]]

Anthony, seems that we aren't getting any traction on this sec-critical bug. Are there others who might be able to look at it?

Comment 13

[Ralph Giles (:rillian)]

I've been on vacation and haven't had a chance to respond. Snorp, what did you think of Anthony's proposal in #7? Part (a) at least should be straightforward.

Comment 14

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to Ralph Giles (:rillian) from comment #13)
> I've been on vacation and haven't had a chance to respond. Snorp, what did
> you think of Anthony's proposal in #7? Part (a) at least should be
> straightforward.

We can probably just pass the MediaResource into the demuxer and check to see if it fails reading the metadata but also fail if we see fragmented mp4. I wouldn't expect that to be difficult but I won't be the person actually doing it.

Comment 15

[James Willcox (:snorp) ([e-mail]) (he/him)]

I does seem relatively straightforward. Eugen can you look into this?

Comment 16

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

Is 43 affected as well? I'll track it for now.

Comment 17

[Ralph Giles (:rillian)]

How's this going, Eugen? Anything I can help with?

Comment 18

[Kevin Brosnan [Ex-Mozilla]]

Liz this issue is independent of the Firefox release version. It is a flaw in Android system decoders that we use.

Comment 19

[Eugen Sawin [:esawin]]

(In reply to Ralph Giles (:rillian) from comment #17)
> How's this going, Eugen? Anything I can help with?

I haven't had time to work on this yet, but we've discussed the issue and are working on enabling click-to-play videos to mitigate the risk for the pre 4.1 users in the meanwhile.

If we want to fix this, we will need a convenient way to read MP4 metadata to detect fragmented streams without sending the stream through the whole media pipeline. Is this something we can easily extract out of the MP4Demuxer?

On a side note, are we certain that only the 3GPP and fMP4 paths are affected (I haven't followed the previously reported MP4 bugs)?

Comment 20

[Ralph Giles (:rillian)]

(In reply to Eugen Sawin [:esawin] from comment #19)

> If we want to fix this, we will need a convenient way to read MP4 metadata
> to detect fragmented streams without sending the stream through the whole
> media pipeline. Is this something we can easily extract out of the
> MP4Demuxer?

We were thinking you could run it through the actual MP4Demuxer in-tree, which should be more robust than an unpatched one. If it rejects the stream, mark it as corrupt. If it can parse the header, pass it to the android decoder for playback.

> On a side note, are we certain that only the 3GPP and fMP4 paths are
> affected (I haven't followed the previously reported MP4 bugs)?

I'm not certain, but these file types are uncommon in HTML5 <video> so it seems an easy way to reduce our exposure without affecting users. fMP4 in particular gives an attacker many headers in which to try an exploit.

Comment 21

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to Ralph Giles (:rillian) from comment #20)
> I'm not certain, but these file types are uncommon in HTML5 <video> so it
> seems an easy way to reduce our exposure without affecting users. fMP4 in
> particular gives an attacker many headers in which to try an exploit.

We don't absolutely need to block fragmented MP4. However it is sufficiently uncommon that we're better to block it than put effort into supporting it. I'm just trying to minimise future work.

Comment 22

[Andrew McCreight [:mccr8]]

(In reply to Eugen Sawin [:esawin] from comment #19)
> I haven't had time to work on this yet, but we've discussed the issue and
> are working on enabling click-to-play videos to mitigate the risk for the
> pre 4.1 users in the meanwhile.

Has this click-to-play been done? What's the bug number for it? Is there any update on the work in this bug? Thanks.

Comment 23

[Eugen Sawin [:esawin]]

With bug 659285 we require user consent for play() invocations with media.autoplay.enabled=false, bug 1166961 adds the custom UI mechanics which allow for playback of blocked videos.

With both patches landed, we can disable auto-play for the affected platforms by default. We think this mitigates the risk since malicious media is unlikely to be served from (user-)trusted sources. That should put us on par with dedicated video-playing apps on those platforms.

Comment 24

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1188240-Disable-auto-playing-of-media-without-us.patch

Comment 25

[Eugen Sawin [:esawin]]

Are we OK with landing this, snorp?

Comment 26

[James Willcox (:snorp) ([e-mail]) (he/him)]

I'm ok with it.

Comment 27

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

I had to back this out in https://hg.mozilla.org/integration/mozilla-inbound/rev/a285afa33378 for some reftest failures:

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

Comment 28

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

Also appears to have broken https://treeherder.mozilla.org/logviewer.html#?job_id=14913291&repo=mozilla-inbound

Comment 29

[Eugen Sawin [:esawin]]

We need to set the pref for remote reftests and mochitests, looks good on try now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=72d9f709189f

Comment 30

[Eugen Sawin [:esawin]]

Attachment: 0002-Bug-1188240-Enable-media-auto-play-for-tests-to-allo.patch

This patch enables auto-play to allow for tests to pass, which use play() to invoke media playback.

Comment 31

[Eugen Sawin [:esawin]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/df9c8be97792
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd625bea9fde

Comment 32

[Eugen Sawin [:esawin]]

According to [1], Android 5 and later versions are affected by two newly discovered vulnerabilities, adding MP3 besides MP4 as attack vectors.

[1] https://blog.zimperium.com/zimperium-zlabs-is-raising-the-volume-new-vulnerability-processing-mp3mp4-media/

Comment 33

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/df9c8be97792
https://hg.mozilla.org/mozilla-central/rev/fd625bea9fde

Comment 34

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

Does this still need uplift to aurora and beta? Shouldn't this have had sec-approval set?

Comment 35

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

Snorp can you fill in to request sec-approval and uplift? I'd like to get this into aurora quickly, if that's what we need to do. We go to build this Thursday for beta 5 and if we need to uplift to beta then we should start this process rolling.

Comment 36

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8665552 [details] [diff] [review]
0001-Bug-1188240-Disable-auto-playing-of-media-without-us.patch


[Security approval request comment]
How easily could an exploit be constructed based on the patch? Not at all

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

Which older supported branches are affected by this flaw? All

If not all supported branches, which bug introduced the flaw? N/A

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Should apply directly

How likely is this patch to cause regressions; how much testing does it need? Not likely

This patch does not fix the security issue, which is caused by the stagefright media library on Android. It does, however, reduce the possibility of a breach by not allowing a malicious media file to play automatically.

There is some additional UI needed when autoplay is disabled which is only in 43, so I think we should only uplift this there.

Comment 37

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Given that this is a sec issue that cannot be easily exploited and we do not have a dot release planned (atm), this will be a wontfix for 41.

Comment 38

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

Comment on attachment 8665552 [details] [diff] [review]
0001-Bug-1188240-Disable-auto-playing-of-media-without-us.patch

Approvals given.

Shouldn't there be a nomination for the Beta branch as well?

Comment 39

[Carsten Book [:Tomcat]]

snorp: should the test also be uplifted ?

Comment 40

[Carsten Book [:Tomcat]]

(In reply to Carsten Book [:Tomcat] from comment #39)
> snorp: should the test also be uplifted ?

err  0002-Bug-1188240-Enable-media-auto-play-for-tests-to-allo.patch  i mean

Comment 41

[Eugen Sawin [:esawin]]

Al, we didn't nominate it for Beta because it depends on bug 1166961 which is riding trains on 43.

Carsten, both patches will need an uplift, otherwise some tests will break.

Comment 42

[Carsten Book [:Tomcat]]

Hi Eugen, the 2nd patch failed to apply:

grafting 306426:fd625bea9fde "Bug 1188240 - Enable media auto-play for tests to allow for script play() invocations. r=snorp"
merging layout/tools/reftest/remotereftest.py
warning: conflicts during merge.
merging layout/tools/reftest/remotereftest.py incomplete! (edit conflicts, then use 'hg resolve --mark')
merging testing/profiles/prefs_general.js

Comment 43

[Eugen Sawin [:esawin]]

Attachment: [Aurora] 0002-Bug-1188240-Enable-media-auto-play-for-mochitests.-r.patch

Rebased for Aurora.

Comment 44

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9529a0efe7e6
https://hg.mozilla.org/releases/mozilla-aurora/rev/446a398ed58e

Comment 45

[Sylvestre Ledru [:Sylvestre]]

Looks like we are not planning to take in 42. Updating the flags.

Btw, why do we have leave-open in the keywords?

Comment 46

[Eugen Sawin [:esawin]]

I'm removing the leave-open keyword, we can reopen or file a new bug to address the new vulnerabilities (comment 32) and track work on a proper fix.

Comment 47

[Sylvestre Ledru [:Sylvestre]]

Please use a new bug in this case. This is way easier to manage for release management & sheriffs.

Comment 48

[Daniel Veditz [:dveditz]]

(In reply to Ritu Kothari (:ritu) from comment #37)
> Given that this is a sec issue that cannot be easily exploited [...]

That's not really true. The question snorp answered was the narrow question about how revealing the patch was, and a pref flip is nowhere near the bug and doesn't mention stagefright. But, the sec issue itself is common to Android and if someone puts it together there are exiting PoCs that could be adapted fairly easily. The template question was not a great fit for this particular bug.

Also note that this is in no way a fix for the bug, it just means the attacker has to get the user to click so they can call .play(). Will reduce the victim pool because of the required interaction, but only somewhat.