1128939 - (CVE-2015-0829) MP4 crash access violation

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[pantrombka]

Attachment: crashowa.mp4

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.93 Safari/537.36

Steps to reproduce:

firefox.exe crashowa.mp4




Actual results:

Firefox 35.0.1 - crash
nightly - tab crash

(b4c.8ec): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Program Files (x86)\Mozilla Firefox\MSVCR100.dll - 
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Program Files (x86)\Mozilla Firefox\xul.dll - 
MSVCR100!wcspbrk+0xa8:
66dc0ab6 660f7f7760      movdqa  xmmword ptr [edi+60h],xmm6 ds:002b:12519000=????????????????????????????????



Expected results:

error: invalid mp4 file

Comment 1

[Benjamin Smedberg]

https://crash-stats.mozilla.com/report/index/73f0abe7-b790-4179-9fad-b25f32150203

mp4_demuxer::MP4Demuxer::DemuxVideoSample() is calling into the CRT at http://hg.mozilla.org/mozilla-central/annotate/940118b1adcd/media/libstagefright/binding/mp4_demuxer.cpp#l261

Comment 2

[Daniel Veditz [:dveditz]]

It appears we're reading off into space here (at a quick glance). How badly are we screwed?

Comment 3

[pantrombka]

(9f0.85c): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
MSVCR100!_VEC_memcpy+0x64:
68400ab6 660f7f7760      movdqa  xmmword ptr [edi+60h],xmm6 ds:002b:124e9000=????????????????????????????????
0:053:x86> .load msec
0:053:x86> !exploitable

!exploitable 1.6.0.0
Exploitability Classification: EXPLOITABLE
Recommended Bug Title: Exploitable - User Mode Write AV starting at MSVCR100!_VEC_memcpy+0x0000000000000064 (Hash=0x9437c8de.0xeea7fe2b)

User mode write access violations that are not near NULL are exploitable.

Comment 4

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

Reporter: please ask Mozilla security staff to set sec-bounty? and don't do it yourself.

Comment 5

[Daniel Veditz [:dveditz]]

pantrombka: are you running a regular release version of Firefox? If so could you let it crash without the debugger and then
 1. submit the crash information to us when the crash reporter shows up
 2. restart firefox and go to about:crashes
 3. click the link for your crash (to make sure it gets processed)
 4. paste the crash link (like comment 1) into this bug.

That will give us more information about your system and what versions of which modules you have (in particular we use some OS libraries to decode MPEG4).

In particular, what version of Firefox are you testing? The idea behind capturing your user agent in comment 0 was to get at that information, but it doesn't really help to know which version of Chrome you used to file the bug, we want to know what crashed. Benjamin got different results than you did, but he tested in a Nightly build and yhou might be using Release, Beta, something really old, who knows.

FWIW I can't reproduce on Mac: I get a message saying the video is corrupt.

Comment 6

[pantrombka]

firefox release from https://www.mozilla.org/en-US/firefox/new/
https://crash-stats.mozilla.com/report/index/97b18f24-7aa9-4715-b290-642272150210

firefox nightly
https://crash-stats.mozilla.com/report/index/683d267a-46f8-4307-a64d-9e6372150210

Comment 7

[Daniel Veditz [:dveditz]]

Of those two the release crash looks exploitable as a first impression, and nightly is a null deref. We may have fixed a bug here, or perhaps only masked it. Thanks, we definitely want to make sure that release crash is fixed!

Comment 8

[Daniel Veditz [:dveditz]]

The next thing to do is
1. see if we can reproduce in Fx35. if so...
  2. does it happen on beta (fx36). if so...
    3. Yikes! we need a fix!
  else find what "fixed" it and make sure it's really a fix
else scratch heads about what's different between our systems and the reporters

Comment 9

[pantrombka]

Android
https://crash-stats.mozilla.com/report/index/57f43313-6014-4bed-a4bd-d3bc52150210

Comment 10

[Bobby Holley (:bholley)]

I can look at this.

Comment 11

[Bobby Holley (:bholley)]

jya and I looked at this - it turns out that the fix in bug 1131861 partially fixes (or at least, papers over) the problem. I could reproduce it in a windows nightly from Feb 2nd, but not latest-trunk.

jya is going to take the bug from here.

Comment 12

[Jean-Yves Avenard [:jya]]

that was bug 1128410. That file has an invalid MaxInputSize of 366 bytes. stagefright was allocating a buffer of that size and used it always assuming that it had enough space.

the first write to that buffer was 584 bytes long.. Writing outside the allocated memory.

Now, we always check that the buffer we're writing too is big enough.

I have only looked to prevent issues in the fragmented mp4 use. I see another potential out of bound write elsewhere. I am going to investigate to check that stagefright checks before writing and will report.

bug 1128410 has already been uplifted to 36

Comment 13

[Jean-Yves Avenard [:jya]]

Ok... for that particular video will now behave as expected (not crash).

However, there is another potential out of bound access in there and a specifically crafted MP4 could mess it all up.

However, the fix is going to be trickier as the buffer allocated can get re-allocated outside stagefright in our MP4Sample code; so can't use the same trick as I did on using a nsTArray backend.

Comment 14

[Jean-Yves Avenard [:jya]]

Attachment: Part1. Ensure we have any space in the media buffer before writing

Ensure our destination buffer has enough space to receive the content. Nothing is done to address the lack of memory, we just error.

Comment 15

[Jean-Yves Avenard [:jya]]

Attachment: Part1. Ensure we have any space in the media buffer before writing

seems that it's enough to test if we have the space to write the 4 bytes size. Who cares about the actual data right?... I've covered the two places it was done. I have 2 patches coming with 1- dynamic on the fly memory allocation and 2- check that we don't read outside our buffer

Comment 16

[Jean-Yves Avenard [:jya]]

Attachment: Make sure we limit read to buffer size and handle error nicely

don't assert when we can't read.

Comment 17

[Jean-Yves Avenard [:jya]]

Attachment: Part3. Allocate media buffer internal memory dynamically

Dynamically allocate MediaBuffer's internal data. Keep a maximum of 1MB as it will be expanded if required. Allocations are fallibles

Comment 18

[Jean-Yves Avenard [:jya]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d9437013657a

Comment 19

[Jean-Yves Avenard [:jya]]

Comment on attachment 8562516 [details] [diff] [review]
Part1. Ensure we have any space in the media buffer before writing

Approval Request Comment
[Feature/regressing bug #]: 1128939
[User impact if declined]: Potential out of bound writes.
[Describe test coverage new/current, TreeHerder]: Tested with various MP4 samples, including video in tests (we have very little mp4 mochitests)
[Risks and why]: Low. The core problem has already been resolved and has been backported to beta. We now always check that we have allocated enough memory before writing to the buffer and will exit gracefully.
[String/UUID change made/needed]: None

Comment 20

[Carsten Book [:Tomcat]]

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

Comment 21

[Sylvestre Ledru [:Sylvestre]]

Jean-Yves, do you know if ESR31 is affected?  merci

Comment 22

[Jean-Yves Avenard [:jya]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #21)
> Jean-Yves, do you know if ESR31 is affected? merci 
 
I just checked the repository, and there's no MP4 support in 31.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d5781c23d498
https://hg.mozilla.org/releases/mozilla-beta/rev/783f63db37da

Comment 24

[Ryan VanderMeulen [:RyanVM]]

This hits conflicts on older Gecko branches due to bug 1128410 not being present. Is this worth trying to rebase and land on b2g32/b2g34?

Comment 25

[Jean-Yves Avenard [:jya]]

Bug 1128410 is the actual fix for this bug... it will need to also be uplifted to those branches..

I can make a combined patch that includes all changes at once, though would be easier to simply uplift 1128410, should apply I think

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/d5781c23d498

Comment 27

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

Comment on attachment 8562556 [details] [diff] [review]
Part3. Allocate media buffer internal memory dynamically

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

::: media/libstagefright/frameworks/av/media/libstagefright/MPEG4Extractor.cpp
@@ +3415,5 @@
>          }
>  
>          int32_t max_size;
>          CHECK(mFormat->findInt32(kKeyMaxInputSize, &max_size));
> +        mBuffer = new MediaBuffer(max_size > 1024*1024 ? 1024*1024 : max_size);

std::min(1024 * 1024, max_size);

@@ +3787,5 @@
>          isSyncSample = smpl->isSync();
>  
>          int32_t max_size;
>          CHECK(mFormat->findInt32(kKeyMaxInputSize, &max_size));
> +        mBuffer = new MediaBuffer(max_size > 1024*1024 ? 1024*1024 : max_size);

same

Comment 28

[Jean-Yves Avenard [:jya]]

Attachment: Ensure we never attempt to write outside allocated buffer

patch for b2g32. There are significant differences between this version of libstagefright and the one in central. I have given it a good review, and with those changes it should be safe. Can't build locally so waiting for try to check:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=011919393d49

Comment 29

[Jean-Yves Avenard [:jya]]

Attachment: Ensure we never attempt to write outside allocated buffer

Patch for b2g34. Compiled locally and tested. 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9ceed17ad570

Comment 30

[Jean-Yves Avenard [:jya]]

Attachment: B2G34. Ensure we never attempt to write outside allocated buffer

update title to make it clear it's for 34

Comment 31

[Jean-Yves Avenard [:jya]]

Attachment: B2G32. Ensure we never attempt to write outside allocated buffer

update title to make it clear it's for 32

Comment 32

[Jean-Yves Avenard [:jya]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/21a1b1631c1b
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/dccaeb191e5f

rillian: this should be uplifted to aurora too... too late for beta (there we'll just crash)

Comment 33

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/21a1b1631c1b
https://hg.mozilla.org/mozilla-central/rev/dccaeb191e5f

Comment 34

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/eafa6c73ef0b
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c0602d519761

Comment 35

[Ralph Giles (:rillian)]

Comment on attachment 8562555 [details] [diff] [review]
Make sure we limit read to buffer size and handle error nicely

Requesting 37 uplift of parts 2 and 3 as well.

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Crashes on invalid video files.
[Describe test coverage new/current, TreeHerder]: Landed on m-c.
[Risks and why]: Risk is low. Part 2 simply rejects more invalid files and is minimal risk. Part 3 rewrites the allocator to be easier to understand. The main risk is that a mistake here would introduce a crash in the new code, but that is unlikely.
[String/UUID change made/needed]: None.

Comment 36

[Ralph Giles (:rillian)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/39c0f3dac604
https://hg.mozilla.org/releases/mozilla-aurora/rev/089b5e6cd72e

Comment 37

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/39c0f3dac604
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/089b5e6cd72e

Comment 38

[pantrombka]

On Windows 7:
https://crash-stats.mozilla.com/report/index/72328bb5-4743-4567-a220-a4e652150307


(934.fbc): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Program Files (x86)\Mozilla Firefox\MSVCR120.dll - 
MSVCR120!errno+0x14b:
675b18c8 f3a5            rep movs dword ptr es:[edi],dword ptr [esi]


0:043:x86> !exploitable

!exploitable 1.6.0.0
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Program Files (x86)\Mozilla Firefox\xul.dll - 
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Program Files (x86)\Mozilla Firefox\mozglue.dll - 
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Program Files (x86)\Mozilla Firefox\mozalloc.dll - 
Exploitability Classification: EXPLOITABLE
Recommended Bug Title: Exploitable - User Mode Write AV starting at MSVCR120!errno+0x000000000000014b (Hash=0xaea2fe1c.0x1a243e8c)

User mode write access violations that are not near NULL are exploitable.