If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.
Bug 1128939 (CVE-2015-0829)

MP4 crash access violation

RESOLVED FIXED in Firefox 36, Firefox OS v2.0

Status

()

Core
Audio/Video
--
critical
RESOLVED FIXED
3 years ago
6 months ago

People

(Reporter: pantrombka, Assigned: jya)

Tracking

({sec-critical})

Trunk
mozilla38
x86_64
Windows 8.1
sec-critical
Points:
---
Bug Flags:
sec-bounty +

Firefox Tracking Flags

(firefox35 wontfix, firefox36+ fixed, firefox37+ fixed, firefox38 fixed, firefox-esr31 unaffected, b2g-v1.4 unaffected, b2g-v2.0 fixed, b2g-v2.1 fixed, b2g-v2.2 fixed, b2g-master fixed)

Details

(Whiteboard: [adv-main36+])

Attachments

(6 attachments, 3 obsolete attachments)

1.05 MB, video/mp4
Details
7.67 KB, patch
kentuckyfriedtakahe
: review+
Details | Diff | Splinter Review
2.19 KB, patch
kentuckyfriedtakahe
: review+
Details | Diff | Splinter Review
9.41 KB, patch
kentuckyfriedtakahe
: review+
Details | Diff | Splinter Review
11.32 KB, patch
Details | Diff | Splinter Review
9.43 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

3 years ago
Created attachment 8558510 [details]
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

3 years ago
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
Component: Untriaged → Video/Audio
Product: Firefox → Core
Version: Firefox 38 → Trunk
It appears we're reading off into space here (at a quick glance). How badly are we screwed?
Flags: needinfo?(ajones)
(Reporter)

Comment 3

3 years ago
(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.
(Reporter)

Updated

3 years ago
Flags: sec-bounty?
(Reporter)

Updated

3 years ago
Severity: normal → critical
Reporter: please ask Mozilla security staff to set sec-bounty? and don't do it yourself.
Assignee: nobody → jyavenard
Flags: needinfo?(ajones)
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 6

3 years ago
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
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!
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
status-firefox35: --- → affected
status-firefox38: --- → ?
Keywords: sec-critical
(Reporter)

Comment 9

3 years ago
Android
https://crash-stats.mozilla.com/report/index/57f43313-6014-4bed-a4bd-d3bc52150210
I can look at this.
Assignee: jyavenard → bobbyholley
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.
Assignee: bobbyholley → jyavenard
(Assignee)

Comment 12

3 years ago
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
(Assignee)

Comment 13

3 years ago
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.
(Assignee)

Comment 14

3 years ago
Created attachment 8562467 [details] [diff] [review]
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.
Attachment #8562467 - Flags: review?(ajones)
Attachment #8562467 - Flags: review?(ajones) → review+
(Assignee)

Comment 15

3 years ago
Created attachment 8562516 [details] [diff] [review]
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
Attachment #8562516 - Flags: review?(ajones)
(Assignee)

Updated

3 years ago
Attachment #8562467 - Attachment is obsolete: true
Attachment #8562516 - Flags: review?(ajones) → review+
(Assignee)

Comment 16

3 years ago
Created attachment 8562555 [details] [diff] [review]
Make sure we limit read to buffer size and handle error nicely

don't assert when we can't read.
Attachment #8562555 - Flags: review?(ajones)
(Assignee)

Comment 17

3 years ago
Created attachment 8562556 [details] [diff] [review]
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
Attachment #8562556 - Flags: review?(ajones)
(Assignee)

Comment 18

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d9437013657a
(Assignee)

Comment 19

3 years ago
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
Attachment #8562516 - Flags: approval-mozilla-beta?
Attachment #8562516 - Flags: approval-mozilla-aurora?
status-firefox35: affected → wontfix
status-firefox36: --- → affected
status-firefox37: --- → affected
status-firefox38: ? → affected
https://hg.mozilla.org/mozilla-central/rev/d9437013657a
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox38: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Jean-Yves, do you know if ESR31 is affected?  merci
Flags: needinfo?(jyavenard)
Attachment #8562516 - Flags: approval-mozilla-beta?
Attachment #8562516 - Flags: approval-mozilla-beta+
Attachment #8562516 - Flags: approval-mozilla-aurora?
Attachment #8562516 - Flags: approval-mozilla-aurora+
status-firefox-esr31: --- → ?
tracking-firefox36: --- → +
tracking-firefox37: --- → +
Keywords: checkin-needed
(Assignee)

Comment 22

3 years ago
(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.
Flags: needinfo?(jyavenard)
https://hg.mozilla.org/releases/mozilla-aurora/rev/d5781c23d498
https://hg.mozilla.org/releases/mozilla-beta/rev/783f63db37da
status-b2g-v1.4: --- → unaffected
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → affected
status-b2g-v2.2: --- → affected
status-b2g-master: --- → fixed
status-firefox36: affected → fixed
status-firefox37: affected → fixed
status-firefox-esr31: ? → unaffected
Keywords: checkin-needed
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?
Flags: needinfo?(jyavenard)
(Assignee)

Comment 25

3 years ago
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
Flags: needinfo?(jyavenard)
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/d5781c23d498
status-b2g-v2.2: affected → fixed
Attachment #8562555 - Flags: review?(ajones) → review+
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
Attachment #8562556 - Flags: review?(ajones) → review+
(Assignee)

Comment 28

3 years ago
Created attachment 8563204 [details] [diff] [review]
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
(Assignee)

Comment 29

3 years ago
Created attachment 8563215 [details] [diff] [review]
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
(Assignee)

Comment 30

3 years ago
Created attachment 8563219 [details] [diff] [review]
B2G34. Ensure we never attempt to write outside allocated buffer

update title to make it clear it's for 34
(Assignee)

Updated

3 years ago
Attachment #8563215 - Attachment is obsolete: true
(Assignee)

Comment 31

3 years ago
Created attachment 8563220 [details] [diff] [review]
B2G32. Ensure we never attempt to write outside allocated buffer

update title to make it clear it's for 32
(Assignee)

Updated

3 years ago
Attachment #8563204 - Attachment is obsolete: true
(Assignee)

Comment 32

3 years ago
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)
Flags: needinfo?(giles)
https://hg.mozilla.org/mozilla-central/rev/21a1b1631c1b
https://hg.mozilla.org/mozilla-central/rev/dccaeb191e5f
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/eafa6c73ef0b
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c0602d519761
status-b2g-v2.0: affected → fixed
status-b2g-v2.1: affected → fixed
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.
Flags: needinfo?(giles)
Attachment #8562555 - Flags: approval-mozilla-aurora?
Attachment #8562555 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/39c0f3dac604
https://hg.mozilla.org/releases/mozilla-aurora/rev/089b5e6cd72e
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/39c0f3dac604
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/089b5e6cd72e
Whiteboard: [adv-main36+]
Alias: CVE-2015-0829
Flags: sec-bounty? → sec-bounty+
(Reporter)

Comment 38

3 years ago
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.

Updated

2 years ago
Group: core-security → core-security-release
Blocks: 1212024
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.