Bug 1128939 (CVE-2015-0829)

MP4 crash access violation

RESOLVED FIXED in Firefox 36

Status

()

defect
--
critical
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: pantrombka, Assigned: jya)

Tracking

({sec-critical})

Trunk
mozilla38
x86_64
Windows 8.1
Points:
---
Dependency tree / graph
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)

Reporter

Description

4 years ago
Posted video 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

4 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

4 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

4 years ago
Flags: sec-bounty?
Reporter

Updated

4 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
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
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

4 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

4 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

4 years ago
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

4 years ago
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

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

Comment 16

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

Comment 17

4 years ago
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 19

4 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?
https://hg.mozilla.org/mozilla-central/rev/d9437013657a
Status: NEW → RESOLVED
Last Resolved: 4 years ago
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+
Assignee

Comment 22

4 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)
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

4 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)
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

4 years ago
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 30

4 years ago
update title to make it clear it's for 34
Assignee

Updated

4 years ago
Attachment #8563215 - Attachment is obsolete: true
Assignee

Comment 31

4 years ago
update title to make it clear it's for 32
Assignee

Updated

4 years ago
Attachment #8563204 - Attachment is obsolete: true
Assignee

Comment 32

4 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)
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+
Whiteboard: [adv-main36+]
Alias: CVE-2015-0829
Flags: sec-bounty? → sec-bounty+
Reporter

Comment 38

4 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

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