Bug 193456 – [iOS] Attempting to open a Keynote document to iCloud.com from iCloud Files causes crash

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Joseph Pecoraro]

Attempting to open a Keynote document to iCloud.com from iCloud Files causes crash

Steps To Reproduce:
1. Log in to icloud.com
2. Open Keynote
3. Tap the Cloud+Arrow icon at the top
4. Tap "Browse" to access iCloud Files
5. Select a "On My iPad" file (create one using Keynote app outside of Safari for example)
  => WebContent crashes

Crash:

Exception Type:  EXC_CRASH (SIGABRT)
Exception Codes: 0x0000000000000000, 0x0000000000000000
Exception Note:  EXC_CORPSE_NOTIFY
Triggered by Thread:  0

Application Specific Information:
*** Terminating app due to uncaught exception 'NSRangeException', reason: '*** -[__NSArrayM objectAtIndexedSubscript:]: index 0 beyond bounds for empty array'
terminating with uncaught exception of type NSException
abort() called

Last Exception Backtrace:
0   CoreFoundation                     0x1a2c3fb08 __exceptionPreprocess + 232 (NSException.m:198)
1   libobjc.A.dylib                    0x1a3144a58 objc_exception_throw + 56 (objc-exception.mm:557)
2   CoreFoundation                     0x1a2ba5174 _CFThrowFormattedException + 112 (CFObject.m:1969)
3   CoreFoundation                     0x1a2b2a418 -[__NSArrayM objectAtIndexedSubscript:] + 180 (NSArrayM.m:299)
4   WebKit                             0x19bc14c74 WebKit::fallbackIconForFile(NSURL*) + 60 (WebIconUtilities.mm:101)
5   WebKit                             0x19bc15164 WebKit::iconForFile(NSURL*) + 160 (WebIconUtilities.mm:157)
6   WebKit                             0x19bdb734c WebKit::WebChromeClient::createIconForFiles(WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul> const&) + 100 (WebChromeClientIOS.mm:155)
7   WebKit                             0x19be5de7c WebKit::WebChromeClient::loadIconForFiles(WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::FileIconLoader&) + 28 (WebChromeClient.cpp:819)
8   WebCore                            0x1a8b3de44 WebCore::FileInputType::setFiles(WTF::RefPtr<WebCore::FileList, WTF::DumbPtrTraits<WebCore::FileList> >&&, WebCore::FileInputType::RequestIcon) + 600 (FileInputType.cpp:393)
9   WebCore                            0x1a8b47768 WTF::Function<void (WTF::Ref<WebCore::FileList, WTF::DumbPtrTraits<WebCore::FileList> >&&)>::CallableWrapper<WebCore::FileInputType::filesChosen(WTF::Vector<WebCore::FileChooserFileInfo, 0ul, WTF::CrashOnOverflow, 16ul> const&, WTF::String const&, WebCore::Icon*)::$_0>::call(WTF::Ref<WebCore::FileList, WTF::DumbPtrTraits<WebCore::FileList> >&&) + 56 (FileInputType.cpp:418)
...

[Joseph Pecoraro]

<rdar://problem/47275642>

[Joseph Pecoraro]

Created attachment 359190 [details]
[PATCH] Proposed Fix

[Wenson Hsieh]

Comment on attachment 359190 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=359190&action=review

> Source/WebKit/Shared/ios/WebIconUtilities.mm:101
> +    if (!interactionController.icons.count)

This check seems a bit strange. According to https://developer.apple.com/documentation/uikit/uidocumentinteractioncontroller/1616801-icons?language=objc, the resulting icons should be non-empty. I wonder if <rdar://problem/37157837> has resurfaced...

[Joseph Pecoraro]

> I wonder if <rdar://problem/37157837> has resurfaced...

Likely. I filed <rdar://problem/47292126> which seems related.

Should we bother trying to protect against this or rely on other components to make a fix?

[Wenson Hsieh]

(In reply to Joseph Pecoraro from comment #4)
> > I wonder if <rdar://problem/37157837> has resurfaced...
> 
> Likely. I filed <rdar://problem/47292126> which seems related.
> 
> Should we bother trying to protect against this or rely on other components
> to make a fix?

Hm...IMO, we should keep this in the backburner, and wait to see if the relevant folks are willing to pull <rdar://problem/47292126> into the relevant release. If that bug is punted, then it's probably for the better to land this as a mitigation (preferably with a comment citing that the check shouldn't be needed once <rdar://problem/47292126> is addressed).

[Joseph Pecoraro]

Created attachment 360210 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Comment on attachment 360210 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=360210&action=review

> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:173
>  (allow mach-lookup
>      (xpc-service-name "com.apple.lsdiconservice") ;; Remove this line after <rdar://problem/47151295> is fixed.
>      (xpc-service-name "com.apple.iconservice"))

I'm not sure if we should get rid of these at this point, but I'm leaving them to be safe.

[Brent Fulgham]

Comment on attachment 360210 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=360210&action=review

> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:175
> +    (global-name "com.apple.iconservices"))

I think the prior line might be a typo: "com.apple.iconservice" -> "com.apple.iconservice".

Can you try just adding the 's' to icon service and see if it fixes it?

[Brent Fulgham]

(In reply to Brent Fulgham from comment #8)
> Comment on attachment 360210 [details]

> I think the prior line might be a typo: "com.apple.iconservice" ->
> "com.apple.iconservice".

See! I did it again!

"com.apple.iconservice" -> "com.apple.iconservices".

[Joseph Pecoraro]

(In reply to Brent Fulgham from comment #8)
> Comment on attachment 360210 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=360210&action=review
> 
> > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:175
> > +    (global-name "com.apple.iconservices"))
> 
> I think the prior line might be a typo: "com.apple.iconservice" ->
> "com.apple.iconservice[s]".

This does not solve the issue.

Note that even though there is:

    (xpc-service-name "com.apple.lsdiconservice")

There is also (further down):

    (global-name "com.apple.lsdiconservice")

---

Would you rather I move the addition:

    (allow mach-lookup
        (global-name "com.apple.iconservices"))

Down into the section at the bottom which has "with report":

    (allow mach-lookup (with report)
        ...)

[Brent Fulgham]

(In reply to Joseph Pecoraro from comment #10)
> (In reply to Brent Fulgham from comment #8)
> > Comment on attachment 360210 [details]
> > [PATCH] Proposed Fix
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=360210&action=review
> > 
> > > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:175
> > > +    (global-name "com.apple.iconservices"))
> > 
> > I think the prior line might be a typo: "com.apple.iconservice" ->
> > "com.apple.iconservice[s]".
> 
> This does not solve the issue.

Okay. Thank you for checking.

[Brent Fulgham]

Comment on attachment 360210 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=360210&action=review

>>>> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:175
>>>> +    (global-name "com.apple.iconservices"))
>>> 
>>> I think the prior line might be a typo: "com.apple.iconservice" -> "com.apple.iconservice".
>>> 
>>> Can you try just adding the 's' to icon service and see if it fixes it?
>> 
>> This does not solve the issue.
>> 
>> Note that even though there is:
>> 
>>     (xpc-service-name "com.apple.lsdiconservice")
>> 
>> There is also (further down):
>> 
>>     (global-name "com.apple.lsdiconservice")
>> 
>> ---
>> 
>> Would you rather I move the addition:
>> 
>>     (allow mach-lookup
>>         (global-name "com.apple.iconservices"))
>> 
>> Down into the section at the bottom which has "with report":
>> 
>>     (allow mach-lookup (with report)
>>         ...)
> 
> Okay. Thank you for checking.

Can you please change the incorrect string:
    (xpc-service-name "com.apple.iconservice")

To the correct service name:
    (xpc-service-name "com.apple.iconservices")

... and then add your new rule for global-name?

(allow mach-lookup
    (global-name "com.apple.iconservices"))

[Brent Fulgham]

(In reply to Joseph Pecoraro from comment #10)
> Would you rather I move the addition down into the section at the bottom which has "with report":

No, please leave it separate. The 'with-report' section is for something Per Arne and I are working on.

Thanks!

[Joseph Pecoraro]

Created attachment 360376 [details]
[PATCH] For Landing

[WebKit Commit Bot]

Comment on attachment 360376 [details]
[PATCH] For Landing

Clearing flags on attachment: 360376

Committed r240606: <https://trac.webkit.org/changeset/240606>