1893891 - (CVE-2024-4771) AddressSanitizer: heap-use-after-free gfx/thebes/gfxFontEntry.cpp in ForgetHashEntry after calloc failure on hb_object_create

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Irvan Kurniawan [:sourc7]]

Attachment: ForgetHashEntry-heap-use-after-free-write-8.txt

On hb_blob_create_or_fail when the hb_object_create<hb_blob_t> calloc return nullptr, after user_data is freed, the user_data able to reused by ForgetHashEntry code.

Steps to reproduce

1. Apply attached hb-blob-create-or-fail.patch
2. Compile Firefox with AddressSanitizer
3. Visit attached testcase-for-hb-blob-patch.html
4. Firefox crashed with AddressSanitizer: heap-use-after-free

Comment 1

[Irvan Kurniawan [:sourc7]]

Attachment: hb-blob-create-or-fail.patch

Comment 2

[Irvan Kurniawan [:sourc7]]

Attachment: testcase-for-hb-blob-patch.html

Comment 3

[Andrew McCreight [:mccr8]]

The jemalloc we use in Firefox is infallible by default. It looks like this allocation is done by calloc. glandium, am I right in thinking that calloc will never return null? Thanks. Though maybe other people build Firefox with other allocators.

Comment 4

[Mike Hommey [:glandium]]

Jemalloc is not infallible. moz_malloc/moz_calloc are.

Comment 5

[Mike Hommey [:glandium]]

That was moz_xmalloc/moz_xcalloc. https://searchfox.org/mozilla-central/rev/1e743db12971e365d1f8fc04c0c52ed55c01494b/memory/mozalloc/mozalloc.h#78-80

Comment 6

[Jonathan Kew [:jfkthame]]

While this is a valid bug, I think the real-world risk here must be pretty low. Note that what's being allocated here is the hb_blob_t struct, which is just 48 bytes (not a possibly-large block with an attacker-controlled size). So if the calloc here does fail, we must be on the very edge of memory exhaustion. In that case it's likely that within moments, an "infallible" allocation attempt is going to terminate the process anyway.

Still, we clearly need to handle the potential failure here more correctly. Or we could switch harfbuzz to use our infallible moz_x* functions and avoid the issue.

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1893891 - Clear mSharedBlobData if blob creation failed.

Comment 8

[Jonathan Kew [:jfkthame]]

I haven't verified this with a local build yet, but if I'm reading the code right, I think all that's needed is to ensure we clear the mSharedBlobData pointer in the failure case. I'll try to confirm this on my Linux machine in a while.

Irvan, if you're able to apply the above patch and check whether it resolves the issue for you, that would be great. Thanks!

Comment 9

[Jonathan Kew [:jfkthame]]

Ok, in a local ASAN build I confirmed that the patch resolves the issue for me. (Further confirmation would still be welcome.)

Comment 10

[Andrew McCreight [:mccr8]]

I'll mark it sec-moderate because it sounds difficult to actually exploit.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c91b19ecb5d3
Clear mSharedBlobData if blob creation failed. r=gfx-reviewers,lsalzman

Comment 12

[Irvan Kurniawan [:sourc7]]

(In reply to Jonathan Kew [:jfkthame] from comment #8)

I haven't verified this with a local build yet, but if I'm reading the code right, I think all that's needed is to ensure we clear the mSharedBlobData pointer in the failure case. I'll try to confirm this on my Linux machine in a while.

Irvan, if you're able to apply the above patch and check whether it resolves the issue for you, that would be great. Thanks!

Thanks Jonathan for the patch, after added mSharedBlobData = nullptr; above return hb_blob_reference(mBlob); then re-compile the Firefox ASan, I confirm that this solves the problem. Thanks!

Comment 13

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/mozilla-central/rev/c91b19ecb5d3

Comment 14

[Donal Meehan [:dmeehan]]

:jfkthame can you attach a beta uplift request on this when ready. Is this something we want to uplift to ESR115?

Comment 15

[Jonathan Kew [:jfkthame]]

I think it would be fine to uplift to ESR, in that the patch carries no significant regression risk; but I also think the issue is likely so hard to exploit (reaching this particular small-allocation failure, without OOM-crashing the process) that the real-world risk is minimal. So not sure it's worth touching ESR here...?

Comment 16

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1893891 - Clear mSharedBlobData if blob creation failed.

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

Comment 17

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: potential UAF after a malloc() failure
• Code covered by automated testing: no
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: minimal
• Explanation of risk level: trivial - clearing a pointer on error
• String changes made/needed: none
• Is Android affected?: yes

Comment 18

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/9d0106534880

Comment 19

[Jesse Schwartzentruber (:truber)]

Attachment: advisory.txt