parent UAF in AudioIPC ServerStreams data_callback from bad content process data
Categories
(Core :: Audio/Video, defect, P2)
Tracking
()
People
(Reporter: dalmurino, Assigned: kinetik)
References
(Depends on 1 open bug)
Details
(4 keywords, Whiteboard: [client-bounty-form][adv-main136+][adv-esr128.8+][adv-esr115.21+])
Attachments
(5 files)
It's a typical scenario of use-after-free. In |process_msg()|1, |ServerStream| is removed from |streams| while handling the |StreamDestroy| message2. And then in |data_callback()|3, |cbs|, which is a member of the |ServerStream| and is stored in |user_ptr|, is accessed without any check. In addition, you can see that the |cbs| is stored into |user_ptr| in 4.
I think that a UAF issue might exist not only at this location but also in other parts where |cbs| is accessed via |user_ptr| like 5, 6, etc.
I attempted to get ASAN report but it didn't work like bug 1902307. From my analysis, it appears that Nightly ASAN Build can trace the deallocation when |ServerStream| is removed. However, ASAN doesn't seem to function when the |cbs| is accessed in |data_callback()|.
Instead, I've attached the callstacks and assembly contexts from the crash.
With patch.diff, you can emulate a compromised Content Process to trigger the UAF. Given that this vulnerability involves a race condition, I've inserted some Sleep statements into both Content Process and Browser Process codebases to increase the success rate.
Although it is not a fundamental solution, I suggest adding some checks to ensure that corresponding |ServerStream| in |streams| has not been removed before accessing |cbs| via |user_ptr|.
REPRODUCTION CASE
Type of vulnerability: Browser Process
Operating System: Win11 x64
To reproduce the crash please follow these steps:
- Apply patch.diff
- Open a video in firefox browser e.g. the attached mp4 file, or video website
// patch.diff emulates a compromised content process and adds some sleeps to improve the success rate
When running this PoC on release build, you can find that a crash happens due to the effect of UAF.
For the exact cause, please refer to the attached call stack file or take a look using a debugger.
Comment 3•1 year ago
|
||
Tyson: any tips on how to get ASAN working to catch this kind of crash? Or do we have trouble capturing these, too?
Updated•1 year ago
|
![]() |
||
Updated•1 year ago
|
![]() |
||
Updated•1 year ago
|
Comment 4•1 year ago
|
||
Sorry I'm not sure if that is possible. Maybe decoder knows?
Assignee | ||
Updated•1 year ago
|
Updated•1 year ago
|
Comment 5•1 year ago
|
||
I don't see a reason why ASan wouldn't report here, as it sounds like a use-after-free like any other.
Comment 6•1 year ago
|
||
Is this another IPC library which is not using IDL?
Assignee | ||
Comment 7•1 year ago
|
||
(In reply to Simon Friedberger (:simonf) from comment #6)
Is this another IPC library which is not using IDL?
Yes, this is a small piece of IPC code specialized for real-time audio requirements.
Assignee | ||
Comment 8•1 year ago
|
||
The root cause is in libcubeb's WASAPI backend, so this bug only affects Windows. The stream destroy logic attempts to handle a non-responsive render thread by abandoning ownership of the cubeb stream to the render thread after a timeout expires while waiting for render thread shutdown. It may have become easier to trigger this UAF since bug 1833633 landed (which removed the "emergency bailout" logic), but the issue seems to have existed ever since the stream destroy timeout was added.
I can't see a safe way to fix this without removing the stream destroy timeout. The non-responsive render thread issue that the stream destroy timeout attempted to work around was mainly present in Windows 8 and earlier (IIRC), so the risk of removing this logic is fairly low now - additionally we have plans in bug 1899317 to move the audio server to a separate process which can be safely restarted if the OS audio backend becomes non-responsive, which is a more robust fix for a non-responsive render thread.
![]() |
||
Updated•1 year ago
|
Comment 9•1 year ago
|
||
The severity field for this bug is set to S3. However, the bug is flagged with the sec-high
keyword.
:kinetik, could you consider increasing the severity of this security bug?
For more information, please visit BugBot documentation.
![]() |
||
Updated•1 year ago
|
Comment 11•8 months ago
|
||
All of the stacks in the callstack attachment look like Rust panics. Are these really potentially-exploitable UAF? They look like Rust detected the problems and has thrown itself on the grenade to save us.
Reporter | ||
Comment 12•8 months ago
|
||
(In reply to Daniel Veditz [:dveditz] from comment #11)
All of the stacks in the callstack attachment look like Rust panics. Are these really potentially-exploitable UAF? They look like Rust detected the problems and has thrown itself on the grenade to save us.
As I recall, this vulnerability is exploitable.
I understand that in the final call stack from the attached 'callstacks.txt', there’s a panic caused by Rust itself and I fully understand the intent of your question.
The first point I’d like to clarify is that this vulnerability doesn’t necessarily have to occur in that specific part of the code. 1 The PoC I provided uses a trick I found while analyzing the audioipc codebase, using the "data_callback_rpc.call(CallbackReq::Data)" functionality. I determined that this would help better control the race timing between the Browser Process and the Content Process. I used this approach to simplify the PoC and better expose the vulnerability. Since this is a UAF vulnerability triggered by a race condition between two threads without any locks, the UAF could occur at different points within the |data_callback()| function.
If this UAF can occur anywhere in the |data_callback()| function, the attacker should choose where to occur. There are several member variables in |cbs|, such as |input_frame_size|, |output_frame_size|, |shm|, |condition|, and |data_callback_rpc|. Among these, the most exploitable seems to be the |shm| used in "self.shm". |shm| is of the type |audioipc2::shm::windows::SharedMem|, and I recall that the ptr could be modified via the UAF. By modifying the ptr in shm, it should be possible to write arbitrary data to an arbitrary address. 2 Therefore, if the PoC were to become more complex, I believe the exploitability would become more apparent.
At the time of reporting, since ASAN wasn’t properly detecting the UAF, I simplified the PoC to prove the UAF. Also, the reason the "callstacks.txt" ends with a Rust panic rather than at the UAF point was to demonstrate that this vulnerability led to an unexpected outcome, using an extreme scenario to illustrate that.
I haven’t reviewed this codebase for a while, but I’ve done my best to summarize the information I remember.
Thank you.
Updated•8 months ago
|
Assignee | ||
Comment 13•7 months ago
|
||
Assignee | ||
Comment 14•6 months ago
|
||
This should be impossible to trigger with the change linked in comment 13. That change has landed via bug 1944150.
Comment 15•6 months ago
|
||
I don't know what branches are affected so I'll assume all of them. Presumably we want to uplift this fix where we can.
Comment 16•6 months ago
|
||
Too late in the cycle to fix this for 135, but we should still get ESR128 and ESR115 patches for the next cycle, yes.
Assignee | ||
Comment 17•6 months ago
|
||
Patches for uplifting to esr115 and esr128 attached to bug 1944150.
Updated•6 months ago
|
Updated•6 months ago
|
Updated•6 months ago
|
Updated•5 months ago
|
Comment 18•5 months ago
|
||
Updated•5 months ago
|
Updated•22 days ago
|
Description
•