1684739 - Crash in [@ objc_msgSend | +[NSEvent isSwipeTrackingFromScrollEventsEnabled]]

Status: RESOLVED FIXED

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

Description

[Gabriele Svelto [:gsvelto]]

Crash report: https://crash-stats.mozilla.org/report/index/63511e22-3bf6-4434-987d-8339a0210102

Reason: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS

Top 10 frames of crashing thread:

0 libobjc.A.dylib objc_msgSend 
1 AppKit +[NSEvent isSwipeTrackingFromScrollEventsEnabled] 
2 XUL nsLookAndFeel::NativeGetInt widget/cocoa/nsLookAndFeel.mm:553
3 XUL nsMediaFeatures::InitSystemMetrics layout/style/nsMediaFeatures.cpp:430
4 XUL Gecko_MediaFeatures_HasSystemMetric layout/style/nsMediaFeatures.cpp:227
5 XUL style::gecko::media_features::MEDIA_FEATURES::__eval servo/components/style/gecko/media_features.rs:591
6 XUL style::media_queries::media_condition::MediaCondition::matches servo/components/style/media_queries/media_condition.rs:173
7 XUL style::media_queries::media_list::MediaList::evaluate servo/components/style/media_queries/media_list.rs:78
8 XUL style::stylist::Stylist::media_features_change_changed_style servo/components/style/stylist.rs:1101
9 XUL Servo_StyleSet_MediumFeaturesChanged servo/ports/geckolib/glue.rs:1823

This appears to be an UAF, the crash address always contains the poison pattern.

Comment 1

[Stephen A Pohl [:spohl]]

This appears to be a duplicate of bug 1370276, which didn't see much activity and was ultimately closed. The +[NSEvent isSwipeTrackingFromScrollEventsEnabled] call seems perfectly legal, so this crash appears to occur in AppKit itself even though this API has been supported since macOS 10.7.

Comment 2

[Stephen A Pohl [:spohl]]

Attachment: Bug 1684739: Correct swipe tracking API name from Swift to Objective-C name. r=haik

Comment 3

[Stephen A Pohl [:spohl]]

It does look like we are using the Swift API name instead of Objective-C. Not sure if this makes any difference though, or why we got past the respondsToSelector check. But it's at least worth giving it a shot.

Comment 4

[Andrew McCreight [:mccr8]]

I'll move this to the layout sec group because of the style stuff in the stack.

Comment 5

[Gabriele Svelto [:gsvelto]]

Note that when the patch lands it will change the crash signature. I'll open a follow-up with the new signature - if it's still crashing.

Comment 6

[Stephen A Pohl [:spohl]]

Well, it looks like we're dealing with a mistake in Apple's documentation now. It appears that the purported Objective-C name isn't actually valid:

2021-01-04 16:03:59.587 firefox[23859:3642004] +[NSEvent swipeTrackingFromScrollEventsEnabled]: unrecognized selector sent to class 0x7fff8831f340
2021-01-04 16:03:59.587 firefox[23859:3642004] Mozilla has caught an Obj-C exception [NSInvalidArgumentException: +[NSEvent swipeTrackingFromScrollEventsEnabled]: unrecognized selector sent to class 0x7fff8831f340]

It looks like we're back to square one and that this is presumably a bug in AppKit itself.

Comment 7

[Gian-Carlo Pascutto [:gcp]]

Marking as stalled because the bug is suspected to be in platform libraries.

Comment 8

[Daniel Veditz [:dveditz]]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #7)

Marking as stalled because the bug is suspected to be in platform libraries.

Have we reported it to Apple? The crash is happening in a really wide range of macos versions from old to new.

Comment 9

[Stephen A Pohl [:spohl]]

I have gone ahead and reported this to Apple as FB9210077:

Please provide a descriptive title for your feedback:
Probable UAF (Use-After-Free) Crash in +[NSEvent isSwipeTrackingFromScrollEventsEnabled]

Which area are you seeing an issue with?
AppKit

What type of issue are you reporting?
Application Crash

Description:
Mozilla has been receiving crash reports from Firefox users about a suspected use-after-free crash in +[NSEvent isSwipeTrackingFromScrollEventsEnabled]. Apple’s documentation doesn’t provide any caution about any situation when it wouldn’t be safe to call this API: https://developer.apple.com/documentation/appkit/nsevent/2870067-swipetrackingfromscrolleventsena?language=objc

This appears to affect all supported macOS versions, from 10.12 to 11.5.

Furthermore, the documentation seems to have the Swift and Objective-C names confused. The Objective-C name should be listed as “isSwipeTrackingFromScrollEventsEnabled” while the Swift name should be “swipeTrackingFromScrollEventsEnabled”.

Here is a sample crash stack from one of our user reports:

Crash Reason 	EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
Crash Address 	0xffffffffe5e5e5f8 (poisoned address)

0 	libobjc.A.dylib 	objc_msgSend
1 	AppKit 	+[NSEvent isSwipeTrackingFromScrollEventsEnabled] 		frame_pointer
2 	XUL 	nsLookAndFeel::NativeGetInt(mozilla::LookAndFeel::IntID, int&) 	widget/cocoa/nsLookAndFeel.mm:553 	frame_pointer
3 	XUL 	nsMediaFeatures::InitSystemMetrics() 	layout/style/nsMediaFeatures.cpp:430 	frame_pointer
4 	XUL 	Gecko_MediaFeatures_HasSystemMetric 	layout/style/nsMediaFeatures.cpp:227 	frame_pointer
5 	XUL 	style::gecko::media_features::MEDIA_FEATURES::__eval 	servo/components/style/gecko/media_features.rs:591 	frame_pointer
6 	XUL 	style::media_queries::media_condition::MediaCondition::matches 	servo/components/style/media_queries/media_condition.rs:173 	frame_pointer
7 	XUL 	style::media_queries::media_list::MediaList::evaluate 	servo/components/style/media_queries/media_list.rs:78 	frame_pointer
8 	XUL 	style::stylist::Stylist::media_features_change_changed_style 	servo/components/style/stylist.rs:1101 	frame_pointer
9 	XUL 	Servo_StyleSet_MediumFeaturesChanged 	servo/ports/geckolib/glue.rs:1823 	frame_pointer
10 	XUL 	mozilla::ServoStyleSet::MediumFeaturesChanged(mozilla::MediaFeatureChangeReason) 	layout/style/ServoStyleSet.cpp:227 	frame_pointer
11 	XUL 	nsPresContext::FlushPendingMediaFeatureValuesChanged() 	layout/base/nsPresContext.cpp:1591 	frame_pointer
12 	XUL 	mozilla::image::VectorImage::MediaFeatureValuesChangedAllDocuments(mozilla::MediaFeatureChange const&) 	image/VectorImage.cpp:1566 	frame_pointer
13 	XUL 	mozilla::dom::ImageTracker::MediaFeatureValuesChangedAllDocuments(mozilla::MediaFeatureChange const&) 	dom/base/ImageTracker.cpp:163 	frame_pointer
14 	XUL 	nsPresContext::MediaFeatureValuesChanged(mozilla::MediaFeatureChange const&, mozilla::MediaFeatureChangePropagation) 	layout/base/nsPresContext.cpp:1565 	frame_pointer
15 	XUL 	nsPresContext::ThemeChangedInternal() 	layout/base/nsPresContext.cpp:1435 	frame_pointer
16 	XUL 	mozilla::detail::RunnableMethodImpl<nsPresContext*, void (nsPresContext::*)(), true, (mozilla::RunnableKind)0, >::Run() 	xpcom/threads/nsThreadUtils.h:1201 	frame_pointer
17 	XUL 	mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) 	xpcom/threads/TaskController.cpp:739 	frame_pointer
18 	XUL 	mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_3>::Run() 	xpcom/threads/nsThreadUtils.h:534 	frame_pointer
19 	XUL 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1200 	frame_pointer
20 	XUL 	NS_ProcessPendingEvents(nsIThread*, unsigned int) 	xpcom/threads/nsThreadUtils.cpp:496 	frame_pointer
21 	XUL 	nsAppShell::ProcessGeckoEvents(void*) 	widget/cocoa/nsAppShell.mm:410 	frame_pointer
22 	CoreFoundation 	__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ 		frame_pointer
23 	CoreFoundation 	__CFRunLoopDoSource0 		frame_pointer
24 	CoreFoundation 	__CFRunLoopDoSources0 		frame_pointer
25 	CoreFoundation 	__CFRunLoopRun 		frame_pointer
26 	CoreFoundation 	CFRunLoopRunSpecific 		frame_pointer
27 	HIToolbox 	RunCurrentEventLoopInMode 		frame_pointer
28 	HIToolbox 	ReceiveNextEventCommon 		frame_pointer
29 	HIToolbox 	_BlockUntilNextEventMatchingListInModeWithFilter 		frame_pointer
30 	AppKit 	_DPSNextEvent 		frame_pointer
31 	AppKit 	-[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] 		frame_pointer
32 	XUL 	-[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] 	widget/cocoa/nsAppShell.mm:170 	frame_pointer
33 	AppKit 	-[NSApplication run] 		frame_pointer
34 	XUL 	nsAppShell::Run() 	widget/cocoa/nsAppShell.mm:673 	frame_pointer
35 	XUL 	nsAppStartup::Run() 	toolkit/components/startup/nsAppStartup.cpp:271 	frame_pointer
36 	XUL 	XREMain::XRE_mainRun() 	toolkit/xre/nsAppRunner.cpp:5109 	frame_pointer
37 	XUL 	XRE_main(int, char**, mozilla::BootstrapConfig const&) 	toolkit/xre/nsAppRunner.cpp:5360 	frame_pointer
38 	firefox 	main 	browser/app/nsBrowserApp.cpp:337 	frame_pointer
39 	libdyld.dylib 	start 		frame_pointer
40 	libdyld.dylib 	start 		scan

Comment 10

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

The severity field for this bug is set to S3. However, the bug is flagged with the sec-high keyword.
:spohl, could you consider increasing the severity of this security bug?

For more information, please visit auto_nag documentation.

Comment 11

[Stephen A Pohl [:spohl]]

Gabriele, could I ask you to take a look here and see if we're still seeing crashes on the latest macOS versions? I can't seem to see any crashes for macOS 12.x, but I'm not sure if this is just because of the way we capture version numbers in our crash reporter, or if this is legitimately not occurring anymore. Apple may have fixed this on their end.

Comment 12

[Gabriele Svelto [:gsvelto]]

I dug into a few similar signatures and it appears that this is still happening, here's a few examples:

• This crash is from macOS 11, has a different signature but contains the isSwipeTrackingFromScrollEventsEnabled method on the stack and has an UAF pattern
• This crash is from macOS 12, also has a different signature but contains the isSwipeTrackingFromScrollEventsEnabled method on the stack and has an UAF pattern

I'm adjusting the crash signatures accordingly, the change was likely triggered by the deployment of our new stack walker. I've also inspected the crashes for useful comments but didn't find any. What I found is that the vast majority of the crashes happened on the about:newtab page which seems odd.

Comment 13

[Stephen A Pohl [:spohl]]

Attachment: Bug 1684739: Only check if swipe tracking is enabled on macOS if the event is a scrollwheel event. r=mstange

Comment 14

[Stephen A Pohl [:spohl]]

Comment on attachment 9269842 [details]
Bug 1684739: Only check if swipe tracking is enabled on macOS if the event is a scrollwheel event. r=mstange

Security Approval Request

• How easily could an exploit be constructed based on the patch?: We do not have steps to reproduce this crash. The patch is a speculative fix that simply rearranges existing checks on NSEvents to first check properties of the events themselves before calling the potentially problematic API to check whether swipe tracking is enabled on the system or not. The patch is unlikely to make construction of an exploit easier in any way.
• 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 older supported branches are affected by this flaw?: 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?:
• How likely is this patch to cause regressions; how much testing does it need?: This patch simply rearranges existing checks and is therefore exceedingly unlikely to cause any regressions.

Comment 15

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 9269842 [details]
Bug 1684739: Only check if swipe tracking is enabled on macOS if the event is a scrollwheel event. r=mstange

Approved to land and uplift if desired

Comment 16

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

Only check if swipe tracking is enabled on macOS if the event is a scrollwheel event. r=mstange
https://hg.mozilla.org/integration/autoland/rev/fa96c2df59d4d78e20314d386a8281bb0dd2a30f
https://hg.mozilla.org/mozilla-central/rev/fa96c2df59d4

Comment 17

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

The patch landed in nightly and beta is affected.
:spohl, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 18

[Stephen A Pohl [:spohl]]

Comment on attachment 9269842 [details]
Bug 1684739: Only check if swipe tracking is enabled on macOS if the event is a scrollwheel event. r=mstange

Beta/Release Uplift Approval Request

• User impact if declined: Possibility of crashes when swiping right/left on a MacBook track pad or magic mouse.
• Is this code covered by automated tests?: No
• 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): This patch is a speculative fix that simply reorders pre-existing checks on scrollwheel events. At worst, there will be no change in crash data. At best, we can address this crash once and for all.
• String changes made/needed: none

Comment 19

[Stephen A Pohl [:spohl]]

To clarify: By answering "Yes" to the question about "Has the fix been verified in Nightly", I'm referring to the fact that no regressions have been reported against this speculative fix and that the crash data has not shown any new crashes after this patch landed, although we can't yet be 100% that the crash is actually fixed since we need more time and data to confirm. Having this patch in beta would accelerate the collection of this data, hence why I think it would make sense to uplift.

Comment 20

[Dianna Smith [:diannaS]]

Comment on attachment 9269842 [details]
Bug 1684739: Only check if swipe tracking is enabled on macOS if the event is a scrollwheel event. r=mstange

Approved for 100.0b5

Comment 21

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1d6438173487

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Hi Stephen, please attach a rebased patch for ESR91 and nominate for approval.

Comment 23

[Stephen A Pohl [:spohl]]

Attachment: Bug 1684739: Only check if swipe tracking is enabled on macOS if the event is a scrollwheel event. r=mstange

Comment 24

[Stephen A Pohl [:spohl]]

Comment on attachment 9273874 [details]
Bug 1684739: Only check if swipe tracking is enabled on macOS if the event is a scrollwheel event. r=mstange

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec-high
• User impact if declined: Possibility of crashes when swiping right/left on a MacBook track pad or magic mouse.
• Fix Landed on Version: 101
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch is a speculative fix that simply reorders pre-existing checks on scrollwheel events. At worst, there will be no change in crash data. At best, we can address this crash once and for all.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9273874 [details]
Bug 1684739: Only check if swipe tracking is enabled on macOS if the event is a scrollwheel event. r=mstange

Approved for 91.9esr.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/3bde71d19b45fab43d38764d7adbde25902fbb43

Comment 27

[Wayne Mery (:wsmwk)]

objc_retain | _NSScrollDeviceTypeForEvent and objc_msgSend | _NSScrollDeviceTypeForEvent are seen in various 91.*
Will check for crashes after 91.9.0 releases

Comment 28

[Stephen A Pohl [:spohl]]

I see one crash in 91.9.0 ESR (https://crash-stats.mozilla.org/report/index/b9cef498-78ed-49c4-a27e-8ff950220513), but the call stack is not what I tried to address here. This particular crash occurs during a separate call to NSEvent.isSwipeTrackingFromScrollEventsEnabled at https://hg.mozilla.org/releases/mozilla-esr91/file/3bde71d19b45fab43d38764d7adbde25902fbb43/widget/cocoa/nsLookAndFeel.mm#l515. I don't see an obvious way to address/eliminate that call. If there is a significant crash volume with this call stack, we should track it in a separate bug and find a dedicated fix for it.

Comment 29

[Wayne Mery (:wsmwk)]

FWIW

• bp-a261ed56-3cad-4e4c-8e63-89a0d0220525 Firefox 91.9.1
• bp-71b5fb1f-712a-4d43-a6cf-bd16e0220517 Thunderbird 91.9.0

The average crash rate (which has been exceedingly low) is unchanged

• https://crash-stats.mozilla.org/signature/?signature=objc_retain%20%7C%20_NSScrollDeviceTypeForEvent&date=%3E%3D2022-02-26T10%3A05%3A00.000Z&date=%3C2022-05-26T10%3A05%3A00.000Z#graphs
• https://crash-stats.mozilla.org/signature/?signature=objc_msgSend%20%7C%20_NSScrollDeviceTypeForEvent&date=%3E%3D2022-02-26T10%3A05%3A00.000Z&date=%3C2022-05-26T10%3A05%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_columns=startup_crash&_sort=-date&page=1#graphs

Comment 30

[Stephen A Pohl [:spohl]]

Ugh. Well, I'm fresh out of ideas here. This call should never fail to begin with. :-/