1991458 - (CVE-2025-13012) AddressSanitizer: heap-use-after-free and attempting-double-free in CrashStatsLogForwarder (potentially due to race-condition)

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Irvan Kurniawan [:sourc7]]

Attachment: log_asan_uaf1_esr140.txt

Summary

When simulating swgl gl.cc new_buf realloc failure as on my previous bug 1975837, it able to generate a lot of Crash Annotation GraphicsCriticalError on log stdout, interestingly in rare case it able to hit AddressSanitizer: heap-use-after-free and attempting-double-free in CrashStatsLogForwarder.

Analysis

From the ASan log and local LLM analysis, the crash is probably caused by unsynchronised access/data race between main thread (T0) and render thread (T52) to a shared std::vector<std::string> that lives in CrashStatsLogForwarder instance, as follow:

1. Render thread (T52)

• Creates a temporary std::string (S₁).
• Stores that string into an element of CrashStatsLogForwarder::mStrings. The vector now owns S₁ and its internal character buffer.

2. Main thread (T0)

• Calls CrashStatsLogForwarder::UpdateStringsVector(const std::string&) with a different string (S₂).
• Inside that function the code assigns (S₂) into the same vector slot that already holds (S₁)
• The assignment also triggers the destructor of the old string (std::string::_Rep::_M_dispose).
• That destructor calls operator delete to free the character buffer that belonged to S₁.

3. Render thread (T52) (later)

• Still iterating over mStrings in UpdateCrashReport() and tries to write S₁ to an std::ostream.
• The stream’s operator<< internally calls std::char_traits<char>::copy, which uses memcpy().
• It now read/copies (S₁) from the dangling buffer that was already freed by the main thread, resulting the ASan "heap‑use‑after‑free".

I'm able to reproduce the UAF and double-free on Firefox ESR140 build (commit 9571910, Sep 27 2025) and Firefox ESR128 build (commit 4442784, Jul 3 2025)

I haven't receive the UAF or double-free report on Firefox Nightly build yet, probably due to new assertion Hit MOZ_CRASH(bug: unable to lock indirect) at gfx/wr/webrender/src/compositor/sw_compositor.rs:164, so it crash early and won't generate a lot Crash Annotation GraphicsCriticalError log.

Comment 1

[Irvan Kurniawan [:sourc7]]

Attachment: log_asan_uaf2_esr140.txt

Comment 2

[Irvan Kurniawan [:sourc7]]

Attachment: log_asan_double_free_esr140.txt

Comment 3

[Andrew McCreight [:mccr8]]

Looks like CrashStatsLogForwarder::UpdateStringsVector doesn't acquire mMutex like CrashStatsLogForwarder::Log does, which seems bad. The latter can call the former, in which case the lock is acquired, but so can two Recv methods. It might be worth looking into the thread locking annotations.

The RecvGraphicsError could be easily triggered from a compromised content process, but I don't know about the other stack.

Comment 4

[Andrew McCreight [:mccr8]]

sec-high might be overkill but I'll err on the side of caution here.

Comment 5

[Glenn Watson [:gw]]

Tim, is the referenced code an area you have any knowledge of? Or do you know who might be a good candidate to look at this?

Comment 6

[Timothy Nikkel (:tnikkel)]

Attachment: (secure)

Comment 7

[Timothy Nikkel (:tnikkel)]

Comment on attachment 9518021 [details]
(secure)

Security Approval Request

• How easily could an exploit be constructed based on the patch?: pretty easy to tell that the problem was a missing lock from the patch
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: easy
• How likely is this patch to cause regressions; how much testing does it need?: very unlikely
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: Yes

Comment 8

[Tom Ritter [:tjr]]

Comment on attachment 9518021 [details]
(secure)

Approved to land and request uplift

FWIW for next time:

How easily could an exploit be constructed based on the patch?:
pretty easy to tell that the problem was a missing lock from the patch
Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?:
No

The answer to the first question is really the answer to the second: 'No, these other things don't tell you, but it's pretty easy to tell we're missing a lock'. Calling out those things specifically is more of a reminder to check for them. The first question is more asking about how easy it is to trigger the bug, or reverse engineer a test case from the patch: Is it a difficult to trigger race condition, is the patch in a wildly different part of the code or is it right at the surface level of IPC or WebIDL? Is the exploit primitive very limited (one byte overwrite of a null byte) or very powerful (general UAF in the main allocation arena).

Comment 9

[Timothy Nikkel (:tnikkel)]

Yes I understood the questions, I was not linking the two answers to those two questions in my head, but I guess it sounded like I was making some link between those two answers for someone reading them?

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e416ceb191fa
https://hg.mozilla.org/integration/autoland/rev/6c159b890505
r=gfx-reviewers,nical,lsalzman

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6c159b890505

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:tnikkel, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox145 to wontfix.

For more information, please visit BugBot documentation.

Comment 13

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: sec-high
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: puts variable access behind lock where it should be
• String changes made/needed: none
• Is Android affected?: yes

Comment 14

[Timothy Nikkel (:tnikkel)]

Attachment: (secure)

Original Revision: https://phabricator.services.mozilla.com/D267389

Comment 15

[Phabricator Automation]

firefox-esr140 Uplift Approval Request

• User impact if declined: sec-high
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: puts variable access behind lock where it should be
• String changes made/needed: none
• Is Android affected?: yes

Comment 16

[Timothy Nikkel (:tnikkel)]

Attachment: (secure)

Original Revision: https://phabricator.services.mozilla.com/D267389

Comment 17

[Phabricator Automation]

firefox-esr115 Uplift Approval Request

• User impact if declined: sec-high
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: puts variable access behind lock where it should be
• String changes made/needed: none
• Is Android affected?: yes

Comment 18

[Timothy Nikkel (:tnikkel)]

Attachment: (secure)

Original Revision: https://phabricator.services.mozilla.com/D267389

Comment 19

[Daniel Veditz [:dveditz]]

We're arguing between moderate and high for this race bug. We've decided to call it "high" on the low end for bounty purposes. We'll grant control of the messages from the child process, but if you can demonstrate the ability for a web content process to cause sufficient swgl errors to reliably trigger this race without a hacked process then we will consider increasing the bounty.

Comment 20

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/3e3fb767b6b1

Comment 21

[Pascal Chevrel (relman team) -> :pascalc]

Timothy, your patch does not apply to the esr115 branch, could your rebase/update it please? Thanks

Comment 22

[Timothy Nikkel (:tnikkel)]

Sorry about that. Updated, should be good now.

Comment 23

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/7797d7c17cea

Comment 24

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/815fbffba313
https://hg.mozilla.org/releases/mozilla-beta/rev/8cd458a5c659

Comment 25

[Tom Schuster]

Attachment: advisory.txt

Comment 26

[Tom Schuster]

Attachment: advisory.txt