1692379 - NSInvalidArgumentException: -[__NSCFString substringWithRange:]: Range {0, 30} out of bounds; string length 1

Status: RESOLVED FIXED

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

Description

[Markus Stange [:mstange]]

Example crash report: https://crash-stats.mozilla.org/report/index/bcb588f1-deaa-49aa-b35d-8d3270201225

93% of the crash reports that contain this exception message are from Thunderbird. However, this may not mean that Firefox encounters this exception less; it could mean that Firefox encounters this exception but then does not end up crashing, so the exception message never makes it into a crash report.

https://crash-stats.mozilla.org/search/?app_notes=~NSInvalidArgumentException%3A%20-%5B__NSCFString%20substringWithRange%3A%5D%3A%20Range%20%7B0%2C%2030%7D%20out%20of%20bounds%3B%20string%20length%201&date=%3E%3D2020-11-12T03%3A30%3A00.000Z&date=%3C2021-02-12T03%3A30%3A00.000Z&_facets=app_notes&_facets=product&_sort=-version&_sort=-date&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform_pretty_version#facet-product

We have a few places where we call substringWithRange: ourselves: https://searchfox.org/mozilla-central/search?q=substringWithRange&path=

Let's add extra checks to them to make sure we don't trigger this exception there.

Comment 1

[Markus Stange [:mstange]]

Here's a report with `Range {0, 15}: https://crash-stats.mozilla.org/report/index/6d23ed84-993b-46eb-a769-4d7610210302

Thrown at stack:
__exceptionPreprocess (in CoreFoundation) + 0xea
objc_exception_throw (in libobjc.A.dylib) + 0x30
-[__NSCFString characterAtIndex:].cold.1 (in CoreFoundation) + 0x0
-[__NSCFString hasPrefix:].cold.1 (in CoreFoundation) + 0x0
-[NSConstantDictionary objectForKey:] (in CoreFoundation) + 0x0
mozilla::widget::IMEInputHandler::GetAttributedSubstringFromRange(_NSRange&, _NSRange*) (in XUL) + 0x40b
-[ChildView attributedSubstringForProposedRange:actualRange:] (in XUL) + 0x3a
-[NSTextInputContext(NSInputContext_WithCompletion) attributedSubstringForProposedRange:completionHandler:] (in AppKit) + 0x87
__55-[NSTextInputContext handleTSMEvent:completionHandler:]_block_invoke_4.534 (in AppKit) + 0x55
__194-[NSTextInputContext tryHandleTSMEvent_attributedString_attributedSubstringForProposedRange_withContext:dispatchCondition:dispatchWork:dispatchFurtherCondition:dispatchFurtherWork:continuation:]_block_invoke (in AppKit) + 0x6e
__55-[NSTextInputContext handleTSMEvent:completionHandler:]_block_invoke.529 (in AppKit) + 0x4a
-[NSTextInputContext tryHandleTSMEvent_attributedString_attributedSubstringForProposedRange_withContext:dispatchCondition:dispatchWork:dispatchFurtherCondition:dispatchFurtherWork:continuation:] (in AppKit) + 0x9d
-[NSTextInputContext handleTSMEvent:completionHandler:] (in AppKit) + 0xd77
_NSTSMEventHandler (in AppKit) + 0x12b
DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) (in HIToolbox) + 0x4e6
SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*) (in HIToolbox) + 0x149
SendEventToEventTargetWithOptions (in HIToolbox) + 0x2d
SendTSMEvent_WithCompletionHandler (in HIToolbox) + 0x17d
__SendTextInputEvent_WithCompletionHandler_block_invoke (in HIToolbox) + 0x1e9
SendTextInputEvent_WithCompletionHandler (in HIToolbox) + 0x466
-[IMKInputSession _postEvent:completionHandler:] (in HIToolbox) + 0x9c
-[IMKInputSession _copyUniCharsForRange:intoBuffer:ofLength:completionHandler:] (in HIToolbox) + 0x1ec
-[IMKInputSession _coreAttributesFromRange:whichAttributes:completionHandler:] (in HIToolbox) + 0x101
-[IMKInputSession attributedSubstringFromRange:completionHandler:] (in HIToolbox) + 0xce
__61-[IMKInputSession imkxpc_attributedSubstringFromRange:reply:]_block_invoke (in HIToolbox) + 0x1e5
__CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__ (in CoreFoundation) + 0xc
__CFRunLoopDoBlocks (in CoreFoundation) + 0x17b
__CFRunLoopRun (in CoreFoundation) + 0x992
CFRunLoopRunSpecific (in CoreFoundation) + 0x1ce
-[IMKInputSessionXPCInvocation invocationAwaitXPCReply] (in HIToolbox) + 0x1d6
-[IMKClient menuWithCompletionHandler:] (in HIToolbox) + 0xa2f
IMKInputSessionGetMenuIntoMenuWithCompletionHandler (in HIToolbox) + 0x84
GetInputMethodInstanceMenu_IntoMenu_WithCompletionHandler (in HIToolbox) + 0x7a
utTryToSetupInputMethodMenu (in HIToolbox) + 0x124
__utSetupInputMethodMenuFromDeferredBlock_block_invoke (in HIToolbox) + 0xc3
__CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__ (in CoreFoundation) + 0xc
__CFRunLoopDoBlocks (in CoreFoundation) + 0x17b
__CFRunLoopRun (in CoreFoundation) + 0x390
CFRunLoopRunSpecific (in CoreFoundation) + 0x1ce
RunCurrentEventLoopInMode (in HIToolbox) + 0x124
ReceiveNextEventCommon (in HIToolbox) + 0x167
_BlockUntilNextEventMatchingListInModeWithFilter (in HIToolbox) + 0x40
_DPSNextEvent (in AppKit) + 0x373
-[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] (in AppKit) + 0x548
-[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in XUL) + 0xb1
-[NSApplication run] (in AppKit) + 0x292
nsAppShell::Run() (in XUL) + 0xab
nsAppStartup::Run() (in XUL) + 0x3c
XREMain::XRE_mainRun() (in XUL) + 0x849
XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) (in XUL) + 0x3df
XRE_main(int, char**, mozilla::BootstrapConfig const&) (in XUL) + 0x85
main (in firefox) + 0x24e
start (in libdyld.dylib) + 0x1

Comment 2

[Markus Stange [:mstange]]

Here's the code:

  NSUInteger compositionLength = mIMECompositionString ? [mIMECompositionString length] : 0;
  if (mIMECompositionStart != UINT32_MAX && mIMECompositionStart >= aRange.location &&
      mIMECompositionStart + compositionLength <= aRange.location + aRange.length) {
    NSRange range = NSMakeRange(aRange.location - mIMECompositionStart, aRange.length);
    NSString* nsstr = [mIMECompositionString substringWithRange:range];

So we're checking whether NSMakeRange(mIMECompositionStart, compositionLength) is fully included inside of aRange, and then we try to cut out a piece of mIMECompositionString.
Isn't this check upside down? If we enter this if, it means that mIMECompositionString is already fully inside the requested range, and there's no need to get only a piece of it.
Shouldn't we check that aRange is fully insideNSMakeRange(mIMECompositionStart, compositionLength) instead?

I think this code currently only works if the two ranges match exactly. Otherwise, we will either not enter the if, or we will throw an exception.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Oh, yes, your investigation must be right.

Comment 4

[Markus Stange [:mstange]]

Attachment: Bug 1692379 - Reverse the range inclusion check, to avoid Objective C exceptions from -[NSString substringWithRange:]. r=masayuki

There are two ranges here:

• The requested range aRange, and
• the IME composition range: [mIMECompositionStart, mIMECompositionStart + [mIMECompositionString length]].

Whenever this function was called with a requested range that included the IME
composition range and was larger than the IME composition range, we would
request an invalid substring, and doing so would trigger an Objective C exception.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a0ee10a962a0
Reverse the range inclusion check, to avoid Objective C exceptions from -[NSString substringWithRange:]. r=masayuki

Comment 6

[Andreea Pavel [:apavel]]

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