sparkle-project / Sparkle

A software update framework for macOS
https://sparkle-project.org
Other
7.44k stars 1.05k forks source link

Sparkle 2.6 can crash in SPUSystemNeedsAuthorizationAccessForBundlePath() #2606

Closed core-code closed 2 months ago

core-code commented 2 months ago

we got a crash report from a user, unfortunately it is without line numbers. but i could fire up a disassembler if it helps

 *** -[NSPlaceholderString initWithFormat:locale:arguments:]: nil argument *** -[NSPlaceholderString initWithFormat:locale:arguments:]: nil argument (null) (
    0   CoreFoundation                      0x000000019eab72ec __exceptionPreprocess + 176
    1   libobjc.A.dylib                     0x000000019e59e788 objc_exception_throw + 60
    2   Foundation                          0x000000019fb24f64 _NSDescriptionWithStringProxyFunc + 0
    3   Foundation                          0x000000019fb3282c +[NSString stringWithFormat:] + 68
    4   Sparkle                             0x000000010285d184 SPUSystemNeedsAuthorizationAccessForBundlePath + 14756
    5   Sparkle                             0x000000010283be78 Sparkle + 15992
    6   Sparkle                             0x0000000102851764 Sparkle + 104292
    7   Sparkle                             0x000000010285111c Sparkle + 102684
    8   Sparkle                             0x000000010283f070 Sparkle + 28784
    9   libdispatch.dylib                   0x000000019e7b0750 _dispatch_call_block_and_release + 32
    10  libdispatch.dylib                   0x000000019e7b23e8 _dispatch_client_callout + 20
    11  libdispatch.dylib                   0x000000019e7c0bb8 _dispatch_main_queue_drain + 988
    12  libdispatch.dylib                   0x000000019e7c07cc _dispatch_main_queue_callback_4CF + 44
    13  CoreFoundation                      0x000000019ea83ad4 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 16
    14  CoreFoundation                      0x000000019ea41258 __CFRunLoopRun + 1996
    15  CoreFoundation                      0x000000019ea40434 CFRunLoopRunSpecific + 608
    16  HIToolbox                           0x00000001a91e419c RunCurrentEventLoopInMode + 292
    17  HIToolbox                           0x00000001a91e3fd8 ReceiveNextEventCommon + 648
    18  HIToolbox                           0x00000001a91e3d30 _BlockUntilNextEventMatchingListInModeWithFilter + 76
    19  AppKit                              0x00000001a229fd68 _DPSNextEvent + 660
    20  AppKit                              0x00000001a2a95808 -[NSApplication(NSEventRouting) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 700
    21  AppKit                              0x00000001a229309c -[NSApplication run] + 476
    22  AppKit                              0x00000001a226a2e0 NSApplicationMain + 880
    23  MacUpdater                          0x000000010212a848 main + 44
    24  dyld                                0x000000019e5da0e0 start + 2360
zorgiepoo commented 2 months ago

Yeah need more information. To disassemble or symbolicate it, ideally you'd have the base/image address of Sparkle in the report and if it's x86_64 or arm64. Is it using Sparkle 2.6.0 and is it using a pre-built binary version? It's not obvious where +[NSString stringWithFormat:] is being called and less obvious of where it's called where the format is nil. Potentially there is also inlining involved.

core-code commented 2 months ago

i think we used the stock 2.6 binary, just re-signed which shouldn't change the offsets, but was unable to recover the image address

we can just close this until we receive more reports of this, hopefully with line numbers

core-code commented 2 months ago

also SPUSystemNeedsAuthorizationAccessForBundlePath() does not even call stringWithFormat: so this looks too fishy to continue investigating at this point

zorgiepoo commented 2 months ago

Yeah, you really need the base/image address of Sparkle at minimum so you can compute the slide. If this is not a full crash log report you won't have that.

zorgiepoo commented 2 months ago

Actually looking at the last backtrace item where we do have an offset

5   Sparkle                             0x000000010283be78 Sparkle + 15992

Issue occurs somewhere in -[SPUBasicUpdateDriver didNotFindUpdateWithLatestAppcastItem:hostToLatestAppcastItemComparisonResult:background:].

I think this issue is the same as the one https://github.com/sparkle-project/Sparkle/pull/2533 fixes (fixes #2524) (fix went in 2.6.1).

SPUSystemNeedsAuthorizationAccessForBundlePath was indeed a bogus lead.

core-code commented 2 months ago

i am unsure if that could have been the case. we have a protection against the evil "app has been moved" situation and immediately throw up an alert offering to relaunch the app.

in any case, going to update the framework to the latest version. thanks.

zorgiepoo commented 2 months ago

In any case localization is the only case where a non-static string should be passed as the format string and that PR change guards against a nil value.