Closed Bug 1179484 (CVE-2015-4477) Opened 9 years ago Closed 9 years ago

libcubeb MediaStream use-after-free

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Version:
38 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla42
Tracking Flags:
Tracking Status
firefox39 --- wontfix
firefox40 + verified
firefox41 + fixed
firefox42 + fixed
firefox-esr31 --- unaffected
firefox-esr38 45+ fixed
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- unaffected
b2g-v2.1S --- unaffected
b2g-v2.2 --- affected
b2g-v2.2r --- affected
b2g-master --- fixed

People

(Reporter: abbGZcvu_bugzilla.mozilla.org, Assigned: padenot)

References

Details

(Keywords: csectype-uaf, sec-critical, Whiteboard: [adv-main40+][adv-esr38.7+])

Attachments

(6 files, 2 obsolete files)

repro.html
(deleted), text/html
Details
tests (to be checked in later)
(deleted), patch
Details | Diff | Splinter Review
patch
(deleted), patch
roc
: review+
Sylvestre
: approval-mozilla-aurora+
Sylvestre
: approval-mozilla-beta+
Sylvestre
: approval-mozilla-release-
abillings
: sec-approval+
Details | Diff | Splinter Review
Updated PoC
(deleted), text/html
Details
Reimplement part of 1190676 to plug it better
(deleted), patch
roc
: review+
ritu
: approval-mozilla-esr38-
Details | Diff | Splinter Review
6d1dc24b3302
(deleted), text/plain
Details
Attached file repro.html (deleted) —
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0 Build ID: 20150525141253 Steps to reproduce: <!doctype html> <script> new AudioContext("telephony").createMediaStreamDestination(); location.href = "javascript:'and now we wait...'"; </script> Actual results: A MediaStream object is created, which is used on three different threads: the main thread, a graph thread and a libcubeb thread. From the source at /dome/media/MediaStreamGraph.h, it appears the code tries to take into account the first two with regard to object life-time. However, I found that simply creating and deleting an object (see repro) will cause the object to be garbage collected at some point, before all references are deleted. This which will cause an access violation at 0x5a5a5a5a - a canary value to detect use-after-free. This seem to happen in the thread created for the wasapi_stream_render_loop function as well as the graph thread. Expected results: No use-after-free. Additional info: The following WinDBG breakpoints may be useful while debugging: bm xul!mozilla::MediaStream::MediaStream ".printf \"thread %X: call %y\\r\\n\", @$tid,@eip;gu;.printf \" return new MediaStream = %X\\r\\n\",@eax;g" bm xul!mozilla::MediaStream::~MediaStream ".printf\"thread %X: call %y MediaStream = %X\\r\\n\",@$tid,@eip,@ecx;g" bm xul!mozilla::MediaStreamGraphImpl::MediaStreamGraphImpl ".printf \"thread %X: call %y\\r\\n\", @$tid,@eip;gu;.printf \" return new MediaStreamGraphImpl = %X\\r\\n\",@eax;g" bm xul!mozilla::MediaStreamGraphImpl::AddStream ".printf \"thread %X: call %y MediaStreamGraphImpl = %X, MediaStream = %X\\r\\n\",@$tid,@eip,@ecx,poi(@esp+4);g" bm xul!mozilla::MediaStreamGraphImpl::RemoveStream ".printf \"thread %X: call %y MediaStreamGraphImpl = %X, MediaStream = %X\\r\\n\",@$tid,@eip,@ecx,poi(@esp+4);g" bm xul!mozilla::MediaStreamGraphImpl::~MediaStreamGraphImpl ".printf\"thread %X: call %y MediaStreamGraphImpl = %X\\r\\n\",@$tid,@eip,@ecx;g" bm xul!mozilla::GraphDriver::GraphDriver ".printf \"thread %X: call %y MediaStreamGraphImpl = %X\\r\\n\",@$tid,@eip,poi(@esp+4);gu;.printf \" return new GraphDriver = %X\\r\\n\",@eax;g" bm xul!mozilla::GraphDriver::~GraphDriver ".printf\"thread %X: call %y GraphDriver = %X\\r\\n\",@$tid,@eip,@ecx;g" bm xul!mozilla::ThreadedDriver::ThreadedDriver ".printf \"thread %X: call %y MediaStreamGraphImpl = %X\\r\\n\",@$tid,@eip,poi(@esp+4);gu;.printf \" return new ThreadedDriver = %X\\r\\n\",@eax;g" bm xul!mozilla::ThreadedDriver::Start ".printf \"thread %X: call %y ThreadedDriver = %X\\r\\n\",@$tid,@eip,@ecx;g" bm xul!mozilla::ThreadedDriver::Stop ".printf \"thread %X: call %y ThreadedDriver = %X\\r\\n\",@$tid,@eip,@ecx;g" bm xul!mozilla::ThreadedDriver::~ThreadedDriver ".printf\"thread %X: call %y ThreadedDriver = %X\\r\\n\",@$tid,@eip,@ecx;g" bm xul!mozilla::AudioCallbackDriver::AudioCallbackDriver ".printf \"thread %X: call %y MediaStreamGraphImpl = %X\\r\\n\",@$tid,@eip,poi(@esp+4);gu;.printf \" return new AudioCallbackDriver = %X\\r\\n\",@eax;g" bm xul!`anonymous*namespace'::wasapi_stream_render_loop ".printf \"thread %X: enter wasapi_stream_render_loop\\r\\n\",@$tid;gu;.printf \"thread %X: exit wasapi_stream_render_loop\\r\\n\",@$tid;g" sx- -c ".printf \"thread %X: Access violation\\r\\n\",@$tid;g" av Here's a dump of the output created by these breakpoints during a crash: thread C88: call xul!mozilla::AudioCallbackDriver::AudioCallbackDriver (66cb9e11) MediaStreamGraphImpl = 16C2E0C0 thread C88: call xul!mozilla::GraphDriver::GraphDriver (66cba3a0) MediaStreamGraphImpl = 16C2E0C0 return new GraphDriver = 16CFB000 thread C88: call xul!mozilla::MediaStream::MediaStream (66cc4384) return new MediaStream = 11646A20 thread C88: call xul!mozilla::MediaStreamGraphImpl::MediaStreamGraphImpl (66cc4513) thread C88: call xul!mozilla::ThreadedDriver::ThreadedDriver (66cba569) MediaStreamGraphImpl = 16C2E8A0 thread C88: call xul!mozilla::GraphDriver::GraphDriver (66cba3a0) MediaStreamGraphImpl = 16C2E8A0 return new GraphDriver = 16C8C860 thread C88: call xul!mozilla::MediaStream::MediaStream (66cc4384) return new MediaStream = 16286CF0 thread C88: call xul!mozilla::MediaStream::MediaStream (66cc4384) return new MediaStream = 11646BB0 thread C88: call xul!mozilla::ThreadedDriver::Start (66cc0435) ThreadedDriver = 16C8C860 thread 908: call xul!mozilla::MediaStreamGraphImpl::AddStream (66cc7548) MediaStreamGraphImpl = 16C2E8A0, MediaStream = 16286CF0 thread E1C: enter wasapi_stream_render_loop thread E1C: call xul!mozilla::MediaStreamGraphImpl::AddStream (66cc7548) MediaStreamGraphImpl = 16C2E0C0, MediaStream = 11646A20 thread E1C: call xul!mozilla::MediaStreamGraphImpl::AddStream (66cc7548) MediaStreamGraphImpl = 16C2E0C0, MediaStream = 11646BB0 thread 908: call xul!mozilla::MediaStreamGraphImpl::RemoveStream (66cda097) MediaStreamGraphImpl = 16C2E8A0, MediaStream = 16286CF0 thread 908: call xul!mozilla::MediaStream::~MediaStream (66cc5c01) MediaStream = 16286CF0 (1cc.908): Access violation - code c0000005 (first chance) thread 908: Access violation (1cc.e1c): Access violation - code c0000005 (first chance) thread E1C: Access violation (1cc.e1c): Access violation - code c0000005 (!!! second chance !!!) eax=00000000 ebx=0009f900 ecx=00000008 edx=00000000 esi=00000002 edi=5a5a5a5a eip=66ce636b esp=0c5dfa14 ebp=16286cf0 iopl=0 nv up ei pl nz na po nc cs=001b ss=0023 ds=0023 es=0023 fs=003b gs=0000 efl=00010202 xul!mozilla::StreamBuffer::FindTrack+0x12: 66ce636b 3b07 cmp eax,dword ptr [edi] ds:0023:5a5a5a5a=???????? Btw. I wonder why Firefox is still using 0x5a to fill freed memory, as this results in addresses that are theoretically reachable by heap-spraying. As a mitigation against exploitation, it would make more sense to spray with a value between 0xC0 and 0xFF. Such values would result in addresses that are too high to be allocated in 32-bit Windows.
FYI. This affects all version from 38 Release up to Nightly.
Note on the repro: it takes a few seconds before the object is deleted, please be patient. If you're using the breakpoints I provided, you can see it happening. If there is no crash immediately after the call to xul!mozilla::MediaStream::~MediaStream, restart Firefox to try again.
Component: Untriaged → Video/Audio
Product: Firefox → Core
Component: Video/Audio → Web Audio
Component: Web Audio → Video/Audio
Hi Matthew -- Can you take this bug? It looks to be in cubeb, and Paul's very busy. Thanks.
Flags: needinfo?(kinetik)
This is not a cubeb bug, this a bug in between the MSG, the Web Audio API, and AudioChannels. Thanks to rr, it was pretty easy to debug, since I could repro on Linux. What is happening here: On the main thread, an AudioContext is created, using the channel "telephony". This creates an MSG, of course, of type "telephony", and puts in the AudioChannel -> MSG table: https://dxr.mozilla.org/mozilla-central/source/dom/media/webaudio/AudioDestinationNode.cpp#361 then, https://dxr.mozilla.org/mozilla-central/source/dom/media/MediaStreamGraph.cpp#3034. At this point, we know that we are going to need an AudioCallbackDriver, since we are creating an AudioContext. We go ahead and directly create the AudioCallbackDriver for this MSG. The AudioCallbackDriver start calling back, computing a few samples maybe, and doing its thing, on the cubeb thread, here a thread from PulseAudio because I'm running Linux. Then, the javascript directly creates a `MediaStreamDestination` from the AudioContext, still on the main thread, this is here: https://dxr.mozilla.org/mozilla-central/source/dom/media/webaudio/AudioContext.cpp#274 Then we go and try to get the DOMMediaStream that we need: https://dxr.mozilla.org/mozilla-central/source/dom/media/webaudio/MediaStreamAudioDestinationNode.cpp#31 Note that we only give two arguments to the constructor, this is the bug. The third argument, which is supposed to be the MSG that we want to create the stream for, is optional, and defaults to `nullptr`. A couple calls below, when initializing the `TrackUnionStream` that is backing the `DOMMediaStream` we return to content for our `MediaStreamAudioDestinationNode`, we figure out that we don't have a valid pointer to an MSG (since it defaulted to `nullptr`, so we get the default one: https://dxr.mozilla.org/mozilla-central/source/dom/media/DOMMediaStream.cpp#298. Calling MediaStreamGraph::GetInstance() without any argument gives you the default MSG (with a AudioChannel of "normal"). This would create another cubeb stream, but in this case, since we don't really know this `TrackUnionStream` is going to be used to back a Web Audio object (and hence need audio output), we just get a normal `SystemClockDriver`. At this point, we have a second MSG calling back into the same `MediaStream`, causing all sorts of problems (the crash that I encountered was slightly different than the reporter, because I was running a debug build). The solution (unlike the problem!) is trivial: we just pass in the right graph when creating the `DOMMediaStream`. Patch + crashtest coming up. (Clearing the NEEDINFO for kinetik since I'm taking this).
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(kinetik)
Assignee: nobody → padenot
Attached patch tests (to be checked in later) (deleted) — — Splinter Review
Somehow this does not work. Maybe I need to do some trickery with an iframe and empty the iframe?
Attached patch patch (deleted) — — Splinter Review
roc, see the writeup in comment 4. I'm also thinking of removing the default argument for the graph to make this bug harder to happen again, I'll post a followup patch.
Attachment #8628675 - Flags: review?(roc)
Comment on attachment 8628675 [details] [diff] [review] patch [Security approval request comment] How easily could an exploit be constructed based on the patch? This is a reported as an UAF, but the crash location and cause changes all the time. It can crash in three different threads depending on timing, and with a variety of causes. I think it would be rather hard to have a reliable path to write an exploit. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? It's not trivial to understand what is going on, no, this is at the intersection of multiple modules and threads. Which older supported branches are affected by this flaw? all of them 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? It's the same patch for all branches. How likely is this patch to cause regressions; how much testing does it need? I'm confident this will not cause regression, this issue is obvious and the fix is simple.
Attachment #8628675 - Flags: sec-approval?
> It can crash in three different threads I thought the main thread freed the object, while either the MSG or cubeb/PulseAudio thread use-after-freed it? > the crash location and cause changes all the time. The threads using the objects are very active and will crash almost immediately after the MediaStream and MediaStreamGraphImpl objects have been freed. Both of these objects will have been overwritten with 5a5a5a5a... at the time they are freed. There is hardly enough time for the memory to be "recycled" for use by a new object in such a short time, making sure that all pointers stored in these objects are 0x5a5a5a5a. This is the cause of most, if not all crashes. The address 0x5a5a5a5a is within range of a heap spray. An attacker might be able to store data at that address that minimizes the risk of causing an access violation when a member property of the freed MediaStream and a MediaStreamGraphImpl objects is used, while at the same time causing a call to any method of these objects through their vftables to run arbitrary code. When code execution is achieved, the code could freeze all other threads to prevent further use of the freed object, thus achieving reasonably reliable code execution. I have not created a PoC to test this theory, I am merely speculating.
Flags: sec-bounty?
Keywords: sec-critical
Sec-approval+ for checking on July 14 or later for trunk. Please make branch patches for Aurora, Beta, and ESR38 at that time. We'll probably want this on ESR31 but I defer to release management there.
status-firefox39: --- → wontfix
status-firefox40: --- → affected
status-firefox41: --- → affected
status-firefox42: --- → affected
status-firefox-esr31: --- → affected
status-firefox-esr38: --- → affected
tracking-firefox40: --- → +
tracking-firefox41: --- → +
tracking-firefox42: --- → +
tracking-firefox-esr31: --- → ?
tracking-firefox-esr38: --- → 40+
Whiteboard: [checkin on 7/14]
Attachment #8628675 - Flags: sec-approval? → sec-approval+
Attached file Proof-of-concept exploit (obsolete) (deleted) —
To investigate if this issue was easily exploited, I created a proof-of-concept exploit. It attempts to prove these three assumptions: * The timing of the use(-after-free) can be controlled by varying the number of MediaStreams. Creating more MediaStreams means the thread will take a longer time to destroy them, which delays the use sufficiently to make sure it happens after the free. This PoC creates 256 MediaStreams and triggers the use-after-free for me in most if not all cases. * A heap spraying targeting the address 0x5a5a5a5a allows an attacker to prevent random crashes when properties are read, and control the instruction pointer when a method is called. * 0x5a is not a good value to overwrite freed memory with, as it can result in addresses that are easily allocated with a heap spray. When loaded in firefox, the exploit should take slightly more than 10 seconds to run, depending on your machine and available RAM. After that, firefox will attempt to execute code at address 0xDEADBEEF, which will crash the process. This address was provided by the PoC and proves control over the instruction pointer. No attempt is made to bypass mitigations or execute arbitrary code. I have opened bug 1182002 to request changing the value 0x5a used to mark freed heap in order to mitigate this kind of exploit.
Attached file Updated PoC (deleted) —
Minor update to PoC code: removed an alert() that was left from debugging.
Attachment #8631502 - Attachment is obsolete: true
Attachment #8628673 - Attachment description: r= → tests (to be checked in later)
Attachment #8628675 - Attachment description: r= → patch
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f989aba3e6e We aren't chemspilling for this bug, so this is wontfix for esr31 (EOL at the end of the current cycle).
status-b2g-v2.0: --- → affected
status-b2g-v2.0M: --- → affected
status-b2g-v2.1: --- → affected
status-b2g-v2.1S: --- → affected
status-b2g-v2.2: --- → affected
status-b2g-v2.2r: --- → affected
status-b2g-master: --- → affected
status-firefox-esr31: affected → wontfix
tracking-firefox-esr31: ? → ---
Flags: in-testsuite?
Whiteboard: [checkin on 7/14]
Please nominate this for Aurora/Beta/esr38 approval when you get a chance.
Flags: needinfo?(padenot)
https://hg.mozilla.org/mozilla-central/rev/9f989aba3e6e
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Comment on attachment 8628675 [details] [diff] [review] patch Approval Request Comment [Feature/regressing bug #]: 984498 [User impact if declined]: potentially exploitable crash [Describe test coverage new/current, TreeHerder]: crash test, to be landed later [Risks and why]: not risky, the fix is very simple [String/UUID change made/needed]: none
Flags: needinfo?(padenot)
Attachment #8628675 - Flags: approval-mozilla-release?
Attachment #8628675 - Flags: approval-mozilla-esr38?
Attachment #8628675 - Flags: approval-mozilla-beta?
Attachment #8628675 - Flags: approval-mozilla-aurora?
Comment on attachment 8628675 [details] [diff] [review] patch Sec-critical, taking it in esr too (and in the pre release channels). Not for the release itself...
Attachment #8628675 - Flags: approval-mozilla-release?
Attachment #8628675 - Flags: approval-mozilla-release-
Attachment #8628675 - Flags: approval-mozilla-esr38?
Attachment #8628675 - Flags: approval-mozilla-esr38+
Attachment #8628675 - Flags: approval-mozilla-beta?
Attachment #8628675 - Flags: approval-mozilla-beta+
Attachment #8628675 - Flags: approval-mozilla-aurora?
Attachment #8628675 - Flags: approval-mozilla-aurora+
My bad, I misread the `hg log` when requesting approvals, and ESR 38 is not affected, since it does not have the patch that introduced the problem. Sorry about that.
Flags: needinfo?(padenot)
Attachment #8628675 - Flags: approval-mozilla-esr38+
Flags: sec-bounty? → sec-bounty+
QA Contact: mwobensmith
Flags: needinfo?(dveditz)
I was able to see the crash on Win7, using Fx39 and both attached PoC files. I was not able to make it crash on Fx40.0b8.
Status: RESOLVED → VERIFIED
status-firefox40: fixed → verified
Whiteboard: [adv-main40+]
I confirm the POC shows control of the process on a Win7 machine: bp-19488adb-f8ad-4f13-a8ed-ffd512150730
Flags: needinfo?(dveditz)
Attachment #8636194 - Attachment description: berendjanwever@gmail.com,4500?,2015-07-01,2015-07-14,2015-07-20,true,,, → berendjanwever@gmail.com,7500?,2015-07-01,2015-07-14,2015-07-20,true,,,
Alias: CVE-2015-4477
Group: core-security → core-security-release
Regression from bug 1073615. Reproduces on ESR 38. [Tracking Requested - why for this release]: sec-critical
Blocks: 1073615
status-b2g-v2.2: unaffected → affected
status-b2g-v2.2r: unaffected → affected
status-firefox-esr38: unaffected → affected
tracking-firefox-esr38: --- → ?
Depends on: 1107534
The patch suggested for uplift mentions a different cause for the regression than comment 24. (See comments 20 and 21) So I am not sure we should uplift this to esr38, at least not for 38.6.0, until we have confirmation that we have a patch that may apply correctly and fix the issue. Karl do you want to take this? Or Paul?
Flags: needinfo?(padenot)
Flags: needinfo?(karlt)
We should not try to stuff it into a release last minute, which means we should start on a patch now if we want to get it into ESR 38.7 (shipping with Fx45)
Attached patch Simplest patch to plug this hole (obsolete) (deleted) — — Splinter Review
This just bails out and applies on esr38.
Flags: needinfo?(padenot)
Attachment #8711659 - Flags: review?(roc)
This is more thorough, and should be reasonably safe. Clearing NI for Karl because he's on vacations and I'll be dealing with this.
Flags: needinfo?(karlt)
Attachment #8711660 - Flags: review?(roc)
Comment on attachment 8711659 [details] [diff] [review] Simplest patch to plug this hole [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: User impact if declined: exploitable crash Fix Landed on Version: 39+ Risk to taking this patch (and alternatives if risky): very low risk String or UUID changes made by this patch: none See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8711659 - Flags: approval-mozilla-esr38?
Comment on attachment 8711660 [details] [diff] [review] Reimplement part of 1190676 to plug it better [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: User impact if declined: exploitable crash Fix Landed on Version: 39+ Risk to taking this patch (and alternatives if risky): a bit more risk than the other, but might plug more holes String or UUID changes made by this patch: none See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8711660 - Flags: approval-mozilla-esr38?
We need only one of those.
Comment on attachment 8711659 [details] [diff] [review] Simplest patch to plug this hole Backporting a fix from 40 to esr38.7, makes sense.
Attachment #8711659 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
(In reply to Paul Adenot (:padenot) from comment #31) > We need only one of those. Hi Paul, is this comment re: filling out one uplift request for both patches? Or are you saying we only need one patch to be uplifted to esr38. Please confirm. Thanks!
(In reply to Ritu Kothari (:ritu) from comment #33) > (In reply to Paul Adenot (:padenot) from comment #31) > > We need only one of those. > > Hi Paul, is this comment re: filling out one uplift request for both > patches? Or are you saying we only need one patch to be uplifted to esr38. > Please confirm. Thanks! Only one of the two patches is necessary, yes. They address the same problem in two ways, one being more complicated but maybe more thorough.
Flags: needinfo?(padenot)
(In reply to Paul Adenot (:padenot) from comment #34) > (In reply to Ritu Kothari (:ritu) from comment #33) > > (In reply to Paul Adenot (:padenot) from comment #31) > > > We need only one of those. > > > > Hi Paul, is this comment re: filling out one uplift request for both > > patches? Or are you saying we only need one patch to be uplifted to esr38. > > Please confirm. Thanks! > > Only one of the two patches is necessary, yes. They address the same problem > in two ways, one being more complicated but maybe more thorough. Thanks! I am just going to take the simpler of the two patches. Hopefully that is low-risk and will still address the sec issue as needed.
Comment on attachment 8711660 [details] [diff] [review] Reimplement part of 1190676 to plug it better I chose the simpler patch so r-'ing this one.
Attachment #8711660 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38-
Attached file 6d1dc24b3302 (deleted) —
Hrm I had inverted a couple lines in the previous patch, here is a better one that implements the original idea.
Attachment #8711659 - Attachment is obsolete: true
Flags: needinfo?(padenot) → needinfo?(cbook)
Hey Wes, comment 41 has an updated patch that needs to land on esr38 branch. Could you please help? Thanks!
Flags: needinfo?(wkocher)
Flags: needinfo?(cbook)
Whiteboard: [adv-main40+] → [adv-main40+][adv-main38.7+]
Whiteboard: [adv-main40+][adv-main38.7+] → [adv-main40+][adv-esr38.7+]
Group: core-security-release
Attachment #8636194 - Attachment description: berendjanwever@gmail.com,7500?,2015-07-01,2015-07-14,2015-07-20,true,,, → berendjanwever@gmail.com,7500,2015-07-01,2015-07-14,2015-07-20,true,,,
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: