1966423 - (CVE-2025-6424) UAF in FontFaceSet::Load

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[ShuMing Chang [:LJP]]

Attachment: poc.zip

Root Cause Analysis

In FontFaceSet::Load, mImpl->FindMatchingFontFaces[1] stores pointers of FontFace in the non-refcounted array nsTArray<FontFace*> faces. This is fine as long as those FontFace instances are not freed in the remaining code of FontFaceSet::Load. Unfortunately, that's not the case here.

already_AddRefed<Promise> FontFaceSet::Load(JSContext* aCx,
                                            const nsACString& aFont,
                                            const nsAString& aText,
                                            ErrorResult& aRv) {
  FlushUserFontSet();

  nsTArray<RefPtr<Promise>> promises;

  nsTArray<FontFace*> faces;
  mImpl->FindMatchingFontFaces(aFont, aText, faces, aRv); // [1]
  if (aRv.Failed()) {
    return nullptr;
  }

  for (FontFace* f : faces) {
    RefPtr<Promise> promise = f->Load(aRv); // [2], call to FontFace::Load
    if (aRv.Failed()) {
      return nullptr;
    }
    if (!promises.AppendElement(promise, fallible)) {
      aRv.Throw(NS_ERROR_FAILURE);
      return nullptr;
    }
  }

  return Promise::All(aCx, promises, aRv);
}

In f->Load(aRv)[2], it calls EnsurePromise[3] and eventually calls mLoaded->MaybeResolve(this)[4] to setup resolution of this in a promise then callback. This is a variant of CVE-2024-9680 where user can intercept the setup procedure by overwriting the getter of FontFace.prototype.then and freeing the associated FontFace instances within the getter. Thus, after returning from the call to f->Load(aRv), the FontFace pointers saved in the faces array could now consist of dangling pointers, and next call to f->Load(aRv) will then trigger a Use-After-Free vulnerability.

Promise* FontFace::Load(ErrorResult& aRv) {
  EnsurePromise(); // [3]

  if (!mLoaded) {
    aRv.Throw(NS_ERROR_FAILURE);
    return nullptr;
  }

  mImpl->Load(aRv);
  return mLoaded;
}

void FontFace::EnsurePromise() {
  if (mLoaded || !mImpl || !GetOwnerGlobal()) {
    return;
  }

  ErrorResult rv;
  mLoaded = Promise::Create(GetOwnerGlobal(), rv);

  if (mImpl->Status() == FontFaceLoadStatus::Loaded) {
    mLoaded->MaybeResolve(this); // [4]
  } else if (mLoadedRejection != NS_OK) {
    mLoaded->MaybeReject(mLoadedRejection);
  }
}

Exploitability

The attacker can exploit this by finding and using another structure to occupy the freed FontFace instance, causing type confusion when f->Load() is called. This could allow attacker to forge the data or even the virtual function table in the structure, resulting in arbitrary code execution in the worst case.

In poc.js we demonstrate a crash on access to 0x4141414141414141, which is caused by FontFaceImpl::Load trying to fetch the virtual function table address of its member.

void FontFaceImpl::Load(ErrorResult& aRv) {
  mFontFaceSet->FlushUserFontSet(); // crashed here

  ...
}

We're unable to find a way to exploit this vulnerability without a address leak. However, a dedicated attacker could possibly find a suitable structure for address leak or direct leak address by another vulnerability to exploit this UAF.

Reproduction

1. Unzip the attachment
2. Run python3 -m http.server 8000
3. Visit http://localhost:8000/poc.html

Environment

Tested on Firefox 138.0.3, Ubuntu 24.04 x86_64

Credit

LJP (@ljp_tw) and HexRabbit (@h3xr4bb1t) from DEVCORE Research Team

Comment 1

[Tyson Smith [:tsmith] (Back Aug 12)]

Attachment: asan-log.txt

Comment 2

[Daniel Holbert [:dholbert]]

Thanks for the bug report and for the helpful analysis! I poked a bit locally. There's some subtlety here around what's actually expected to happen.

Chrome and Safari both allow then to be redefined (like in the PoC), and they do call the redefined then under some conditions, BUT they don't call it from FontFaceSet::Load.

I'll attach a more-reduced testcase to demonstrate.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 1

Here's a reduced testcase which signals its results via logging and an alert rather than crashing.

In Firefox, this testcase logs the following:

About to kick off font load...
This is the custom 'then' implementation!
Done

(It signals the second message via an alert() as well to make itself extra clear).

Whereas in Chrome/Safari, there's no logging or alert from the custom then implementation.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 2 (calls font.load().then(...) to invoke custom then function in other browsers too)

(In reply to Daniel Holbert [:dholbert] from comment #2)
Chrome and Safari both allow then to be redefined (like in the PoC), and they do call the redefined then under some conditions, BUT they don't call it from FontFaceSet::Load.

Here's a testcase to illustrate this. Chrome and Safari both invoke the custom then implementation here when it's called more directly via font.load().then(...) (via FontFace::Load rather than FontFaceSet::Load).

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 3 (just calls `font.load()` without directly invoking `then`)

Here's a 3rd variant. This one invokes the custom then implementation in all 3 browsers (just by virtue of having called FontFace::Load), but there's one interesting/important difference that's relevant to this bug:

• In Firefox, the custom then implementation gets invoked during the call to load().
• In Safari and Chrome, the custom then implementation gets invoked asynchronously, i.e. after load() returns and we execute the logging ("Finished calling FontFace::Load!") on the next line and return execution to the event loop.

Comment 6

[Daniel Holbert [:dholbert]]

FWIW the spec describes...
FontFaceSet::Load here:
https://drafts.csswg.org/css-font-loading/#dom-fontfaceset-load
FontFace::Load here:
https://drafts.csswg.org/css-font-loading/#dom-fontface-load

Both of the descriptions have an early step that says to do most the steps asynchronously, which we sorta don't do. (In FontFaceSet::Load, we synchronously call Load on all the FontFace objects, which the spec describes as something to be done as part of the asynchronous task. In FontFace::Load, we resolve the promise synchonously in some cases, and the spec doesn't explicitly say when to resolve it, but it does say to do a bunch of work (maybe including that) inside a "queue a task..." asynchronous step.

If we followed the spec's implied resolve-the-promise-asynchronously suggestion for the FontFace::Load internals, then I think we'd do the correct/safe thing here, and we'd match other browsers on the event-ordering in my just-attached "reduced testcase 3".

Comment 7

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #6)

If we followed the spec's implied resolve-the-promise-asynchronously suggestion for the FontFace::Load internals, then I think we'd do the correct/safe thing here, and we'd match other browsers on the event-ordering in my just-attached "reduced testcase 3".

This^ is probably the best solution ultimately, but it carries some risk since it'll change timing of when events fire, and the added delays might introduce some perf impact (though maybe/probably negligible).

For now I tend to think we should just upgrade the local nsTArray<FontFace*> faces; array to be RefPtrs, since these FontFace objects are refcounted anyway, which gives us that option. That'll add a bit of refcount traffic which is a nonzero cost, but it gives us a pretty safe/obvious band-aid to mitigate the immediate problem here.

Then in the future, if/when we defer the FontFace::Load work to happen later as I suggest in comment 6, we can relax faces to be storing raw pointers again to save ourselves some refcounting traffic.

Comment 8

[Daniel Holbert [:dholbert]]

Incidentally I found two crash reports in mozilla::dom::FontFace::EnsurePromise that might be this same bug:
bp-4056b778-c397-49ad-815a-c2d480250513
bp-f85cf24d-ce48-40c5-a2b0-152760250509

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: Bug 1966423: Hold stronger references to FontFaces. r?emilio

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: Bug 1966423 followup: Use SetCapacity to preallocate space for promises in FontFaceSet::Load. r?emilio,jfkthame

This patch doesn't change behavior; it just does a single preallocation to
avoid wasting time on repeated allocations (and allocation-failure-checks) as
we incrementally append to an array.

Comment 11

[Daniel Holbert [:dholbert]]

I think we can assume all branches are affected here [marking as such]. Details below.

In my test VM, I was able to confirm the POC crashes Firefox Nightly builds as old as 2024-01-01 (version 123.0a1) -- I tested that one and a few newer builds as well and was able to crash in all of them -- so that likely means esr128 and current release/beta are affected.

I haven't yet been able to test esr115 -- it won't launch in my VM for some reason -- but the patch applies cleanly to it, and the FontFaceSet::Load mechanics don't appear to have been touched very much for ages, so I think it's best to assume esr115 is also affected for now.

Comment 12

[Daniel Holbert [:dholbert]]

Comment on attachment 9488837 [details]
Bug 1966423: Hold stronger references to FontFaces. r?emilio

Security Approval Request

• How easily could an exploit be constructed based on the patch?: not easily, but not too difficult either. An expert analysis of the patch could arrive at the insights shared in comment 0 here.
• 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 branches. yes, release status flags accurately reflect that.
• 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?: The patch applies cleanly all the way back to esr115 even.
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely to cause regressions; doesn't require much testing. It just adds strong references to keep an array of objects alive longer. Doesn't change behavior aside from that lifetime-extension.
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: Yes

Comment 13

[Daniel Holbert [:dholbert]]

I recognize that Firefox 139 ships in 1 week. Given that, I think we can consider 138 'wontfix'; no need to ship yet another dot-release for that channel in its last few days of existence, when another update is coming out soon.

I'll defer to sec/uplift-approval folks on when the best time would be for landing/uplifting to other branches, to minimize exposure risk.

Comment 14

[Daniel Holbert [:dholbert]]

(also for the record, I only plan to request uplift for the first patch. The second patch is just a nonfunctional cleanup and is not important to uplift.)

Comment 15

[Daniel Veditz [:dveditz]]

I can confirm ESR 115.x is affected: bp-19eeac47-627d-4726-958b-f01b40250520
Also 128.10: bp-d9bdcc1d-1832-4fee-a44c-42dc80250520 (not a surprise given comment 11)

Both ESRs crash on our UAF poison address (+offset) 0xe5e5e5e5e5e5e625 rather than the "AAAAAAAA" (+offset) from the POC's "payload" that I see in Nightly. That doesn't mean the ESRs are "safe"; it's almost certainly possible to work around whatever has changed since then.

Unfortunately we have already built Release Candidates for 139, and the fact that ESRs need fixing too makes it unlikely to be accepted in 139 point-release. The patch pretty clearly says "UAF in FontFaceSet::Load" so we might want to wait a bit into the 141 nightly cycle before landing and then uplifting to 140.

Comment 16

[Tom Ritter [:tjr]]

Comment on attachment 9488837 [details]
Bug 1966423: Hold stronger references to FontFaces. r?emilio

Approved to land and request uplift 6/6

Comment 17

[Daniel Holbert [:dholbert]]

Attachment: Bug 1966423: Hold stronger references to FontFaces. r?emilio

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

Comment 18

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: open to use-after-free security vulnerability
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: Run steps in comment 0, with the attached poc.zip file
• Risk associated with taking this patch: Low
• Explanation of risk level: This patch just keeps some objects alive a bit longer (preventing us from deleting them before their last use)
• String changes made/needed: None
• Is Android affected?: yes

Comment 19

[Daniel Holbert [:dholbert]]

Attachment: Bug 1966423: Hold stronger references to FontFaces. r?emilio

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

Comment 20

[Phabricator Automation]

firefox-esr128 Uplift Approval Request

• User impact if declined: users are left vulnerable to a use-after-free security vulnerability
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: Run steps in comment 0, with the attached poc.zip file
• Risk associated with taking this patch: Low
• Explanation of risk level: This patch just keeps some objects alive a bit longer (preventing us from deleting them before their last use)
• String changes made/needed: None
• Is Android affected?: yes

Comment 21

[Daniel Holbert [:dholbert]]

Attachment: Bug 1966423: Hold stronger references to FontFaces. r?emilio

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

Comment 22

[Phabricator Automation]

firefox-esr115 Uplift Approval Request

• User impact if declined: users are left vulnerable to a use-after-free security vulnerability
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: Run steps in comment 0, with the attached poc.zip file
• Risk associated with taking this patch: Low
• Explanation of risk level: This patch just keeps some objects alive a bit longer (preventing us from deleting them before their last use)
• String changes made/needed: None
• Is Android affected?: yes

Comment 23

[Daniel Holbert [:dholbert]]

I've verified that the diff ( https://mozphab-phabhost-cdn.devsvcprod.mozaws.net/file/data/47oxma5i47evx52cqsju/PHID-FILE-yf6iz5itkh6ywd26irzb/D250114.1749231971.diff ) applies cleanly to ESR115, ESR128, and beta140.

Comment 24

[BugBot [:suhaib / :marco]]

9 days ago, tjr placed a reminder on the bug using the whiteboard tag [reminder-landing 2025-06-06] .

dholbert, please refer to the original comment to better understand the reason for the reminder.

Comment 25

[Daniel Holbert [:dholbert]]

I triggered Lando.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/7e9c62c30660
https://hg.mozilla.org/integration/autoland/rev/f0a793b929b9
Hold stronger references to FontFaces. r=emilio
https://github.com/mozilla-firefox/firefox/commit/094ddb8fc2c1
https://hg.mozilla.org/integration/autoland/rev/b61ab4095961
followup: Use SetCapacity to preallocate space for promises in FontFaceSet::Load. r=emilio

Comment 27

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/mozilla-central/rev/f0a793b929b9
https://hg.mozilla.org/mozilla-central/rev/b61ab4095961

Comment 28

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/0e47959fc55a
https://hg.mozilla.org/releases/mozilla-beta/rev/987a3a2b92ec

Comment 29

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/3d89c628f2f1

Comment 30

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/f99f3f754033

Comment 31

[Daniel Holbert [:dholbert]]

I verified that this is fixed in Nightly 2025-06-07, using the attached poc.zip and steps at the end of comment 0.

Nightly 2025-06-06 is BAD. (usually -- but not quite always -- triggers a content-process crash when loading poc.html)
Nightly 2025-06-07 is GOOD. (was not able to trigger a content-process crash when loading poc.html)

If I narrow further using mozregression --find-fix, I land on the autoland push in comment 26.

(Additional QA verification is welcome; I wanted to do a quick first-pass verification myself as well, though.)

Note: I was testing in an Ubuntu 24.10 VM, and I had to use Xorg rather than Wayland (selectable via a cog at bottom-right of login screen) in order to avoid tripping bug 1970275 (which causes startup crashes for the past week or so of Nightly builds, so when running in a VM using wayland, for me at least).

Comment 32

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Hello! I can reliably reproduce the crash by opening the attached PoC and following the steps in comment 0 with Firefox 140.0a1 (2025-05-14) on Windows 10x64. If the crash doesn't occur immediately, refreshing the page several times will eventually cause the tab to crash.
The issue is verified fixed with 141.0a1 (2025-06-11), 140.0b8, 128.12.0esr (20250609120909) and 115.25.0esr (20250609120707) on Windows 10x64, macOS 12 and Ubuntu 24.04. No crash occurs after following the steps from comment 0 and refreshing the POC page multiple times.

Comment 33

[Tom Ritter [:tjr]]

Attachment: advisory.txt