1568862 - Crash in [@ objc_msgLookupSuper2]

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P1)

Description

[Marcia Knous [:marcia]]

Seen while looking at nightly crash triage. Both of these crash signatures are similar and have what looks to be potential UAF signatures. The crashes started in 20190724220024: https://bit.ly/2yeXn4M. Not sure where to bucket this one based on the stacks, some of which have image related stuff in them.

Possible regression range based on build id: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=69ac304560c98a733d44a0245fe9782dc6a465e2&tochange=1da5a37de0fd1695e96a9d5e7f59fbd84d8759b2

Comment 1

[Daniel Veditz [:dveditz]]

Interesting that ignoring a couple random Fx52 crashes, this seems to have started in Fx67 AND ESR-60.7. Some security fix we uplifted? A build tool change?

@ _object_remove_assocations
60.7: bp-735d824f-e92f-459e-b976-c91b50190719
67: bp-f4a160dc-be01-4edd-acd9-932030190722

All these crashes seem to be on Mac 10.14 or 10.15

Comment 2

[Marcia Knous [:marcia]]

Looks as if Matthew N hit one of the signatures in Bug 1569012. Timothy believes it is a regression from Bug 1521893. adding ni on Harry here as well because the spike in 70 seems to have started when he landed Bug 1521893. Would be interesting to know why we see crashes before that.

Comment 3

[Harry Twyford]

Looks like the crash signature is converging with bug 1569012. Marcia, would you say this bug's security flag makes it different enough to keep open? Otherwise it could probably be marked as a duplicate.

That said, I agree it's interesting that this crash affects Fx67 -- in bug 1569012, it seemed like this is a result of a patch I pushed to 70. The 67 crash complicates things a little.

Comment 4

[Marcia Knous [:marcia]]

Harry - I think we should track all the signatures in this bug based on the large number of UAF signatures and the fact that it is rated a sec-high bug. Usually if it is a security issue we work in the security bug and not in a open one.

Comment 5

[Marcia Knous [:marcia]]

The combination of these 4 signatures in 70 nightly is making this a topcrash. Adding the relevant keyword.

Comment 7

[Harry Twyford]

Discussion in bug 1569012 is moving towards backing out https://hg.mozilla.org/mozilla-central/rev/4aead1eca8c65fab12771084a3be639564fc1b61 until this can be fixed. Who's the best person to needinfo about that? And is it best to do that here or over in bug 1569012?

Comment 8

[Marcia Knous [:marcia]]

(In reply to Harry Twyford [:harry] from comment #7)

Discussion in bug 1569012 is moving towards backing out https://hg.mozilla.org/mozilla-central/rev/4aead1eca8c65fab12771084a3be639564fc1b61 until this can be fixed. Who's the best person to needinfo about that? And is it best to do that here or over in bug 1569012?

Sheriffs should be able to help us back that out - I can ping them in the IRC channel. I will add Matthew N and Timothy to this bug so that they are aware the signatures are UAFs.

Comment 9

[Marcia Knous [:marcia]]

Adding Aryx - Can you work with Harry to help us back out the changeset in Comment 8?

Comment 10

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

Backed out part 2 of bug 1521893 to fix crashes (Bug 1568862):

https://hg.mozilla.org/integration/autoland/rev/4b23bb4e274c0c0d69b7be2d3cd6090a971a837e

Comment 12

[Harry Twyford]

Attachment: Bug 1568862 - Resolve dangling pointer and faulty error logic in macOS icon loaders. r?spohl!

Comment 13

[Harry Twyford]

The presence of nsMenuItemIconX::OnComplete in the crash signatures is very interesting. That method was only touched in the patch preceding the one that was backed out, and has been landed for two weeks longer: https://hg.mozilla.org/integration/autoland/rev/ab282375f89b

I took a look at it and realized there was some faulty logic in its error handling. I'm not entirely sure this will resolve the above issues, but it's a fix nonetheless!

Comment 14

[Harry Twyford]

Following the Security Approval Guide I'm trying to set the sec-approval flag but can't find it for the life of me. Maybe I'm missing a step? Marcia, can you please advise? The patch on Phabricator is approved and good-to-go.

Comment 15

[Marcia Knous [:marcia]]

(In reply to Harry Twyford [:harry] from comment #14)

Following the Security Approval Guide I'm trying to set the sec-approval flag but can't find it for the life of me. Maybe I'm missing a step? Marcia, can you please advise? The patch on Phabricator is approved and good-to-go.

It's on the patch in the same spot as other approval flags. That being said, if it only affects Nightly, it doesn't need sec-approval.

Comment 16

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

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