1929911 - AddressSanitizer: heap-use-after-free [@ mozilla::gfx::RecordedPopLayer::PlayEvent] with READ of size 8

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Akmat Suleimanov [:asuleimanov]]

The attached testcase crashes on mozilla-central revision 12d31a1006e7d7e9f4b4199e8cb0e93bdae777d5 (build with --enable-fuzzing & moz2d target patch).

For detailed crash information, see attachment.

To reproduce the issue:

1. Build an ASan --enable-fuzzing build including gtests with https://phabricator.services.mozilla.com/D186833 applied.
2. Run FUZZER=Moz2D objdir/dist/bin/firefox test.bin

Comment 1

[Akmat Suleimanov [:asuleimanov]]

Attachment: Detailed Crash Information

Comment 2

[Jesse Schwartzentruber (:truber)]

Attachment: test.bin

Comment 3

[Andrew McCreight (out until 2026) [:mccr8]]

I'll mark this sec-high for now, though of course it could turn out to be something specific to the fuzzing harness.

Comment 4

[Bob Hood [:bhood]]

Lee, can you provide any insights on this one?

Comment 5

[Lee Salzman [:lsalzman]]

I don't understand and am sincerely confused. Why do I have to build in a patch and run a non-descript, untrusted binary file to test what is nominally a sec-high bug? This seems somewhat sketchy.

I would request a safer, more trustworthy testcase before I feel confident in any way engaging with this?

Comment 6

[Tyson Smith [:tsmith]]

(In reply to Lee Salzman [:lsalzman] from comment #5)

Why do I have to build in a patch

This is a fuzzer that is currently in development.

This seems somewhat sketchy.

It uses the libfuzzer interface[1] like many other existing fuzzers[2].

and run a non-descript, untrusted binary file to test what is nominally a sec-high bug?

I believe there are tools in development to convert the binary blob into something human readable and viscera but I'm not sure they are available :truber would know.

I would request a safer, more trustworthy testcase before I feel confident in any way engaging with this?

In the mean time I can get a Pernosco session.

[1] https://firefox-source-docs.mozilla.org/tools/fuzzing/fuzzing_interface.html
[2] https://searchfox.org/mozilla-central/search?q=MOZ_FUZZING_INTERFACE&path=&case=false&regexp=false

Comment 7

[Tyson Smith [:tsmith]]

Oh the tools to convert the blob are in the patch

Comment 8

[Lee Salzman [:lsalzman]]

Due to stated reservations, I am just going to make a reasonable guess at what the issue here is and a proposed patch. Someone else who is more comfortable with handling the binary blobs is welcome to test it.

Comment 9

[Lee Salzman [:lsalzman]]

Attachment: Bug 1929911 - DT fix. r?aosmond

Comment 10

[Tyson Smith [:tsmith]]

I tested the patch and can verify the issue is no longer reproducible, thanks!

Comment 11

[Lee Salzman [:lsalzman]]

Comment on attachment 9436279 [details]
Bug 1929911 - DT fix. r?aosmond

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Not easily.
• 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?:
• If not all supported branches, which bug introduced the flaw?: 123+
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely.
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: Yes

Comment 12

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 9436279 [details]
Bug 1929911 - DT fix. r?aosmond

Approved to land and request uplift

Comment 13

[Lee Salzman [:lsalzman]]

Attachment: Bug 1929911 - DT fix. r?aosmond

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

Comment 14

[Lee Salzman [:lsalzman]]

Comment on attachment 9436279 [details]
Bug 1929911 - DT fix. r?aosmond

Beta/Release Uplift Approval Request

• User impact if declined/Reason for urgency: Possible use-after-free in any rendering - WebRender and Canvas.
• Is this code covered by automated tests?: Unknown
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Avoids holding onto erroneous weak reference. Should be a relatively harmless change.
• String changes made/needed:
• Is Android affected?: Yes

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined:
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/288e8d4633ef
DT fix. r=aosmond

Comment 16

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

https://hg.mozilla.org/mozilla-central/rev/288e8d4633ef

Comment 17

[Donal Meehan [:dmeehan]]

Comment on attachment 9436279 [details]
Bug 1929911 - DT fix. r?aosmond

Approved for 133.0b7

Comment 18

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/785aaa106795

Comment 19

[Donal Meehan [:dmeehan]]

Comment on attachment 9436279 [details]
Bug 1929911 - DT fix. r?aosmond

Approved for 128.5esr.

Comment 20

[Pulsebot]

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