Bug 226546

Summary: [iOS] Fix IOKit sandbox violations
Product: WebKit Reporter: Per Arne Vollan <pvollan>
Component: WebKit Misc.Assignee: Per Arne Vollan <pvollan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Per Arne Vollan 2021-06-02 11:37:46 PDT
Fix IOKit sandbox violations in the WebContent process on iOS.
Comment 1 Per Arne Vollan 2021-06-02 11:38:11 PDT
<rdar://78523469>
Comment 2 Per Arne Vollan 2021-06-02 11:47:49 PDT
Created attachment 430379 [details]
Patch
Comment 3 Per Arne Vollan 2021-06-02 12:04:56 PDT
Created attachment 430386 [details]
Patch
Comment 4 Tim Horton 2021-06-02 12:09:53 PDT
Comment on attachment 430386 [details]
Patch

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

> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb.in:267
> +#if __IPHONE_OS_VERSION_MIN_REQUIRED <= 150000

1) `<= a major version` is highly unusual and likely to lead to errors in the very near future. Does this need a FIXME + radar about reenabling it for future point updates?
2) what about the iOS-derived platforms that use this sandbox file but do not use __IPHONE_OS_VERSION_MIN_REQUIRED?
Comment 5 Per Arne Vollan 2021-06-02 12:33:25 PDT
Created attachment 430391 [details]
Patch
Comment 6 Per Arne Vollan 2021-06-02 12:34:07 PDT
(In reply to Tim Horton from comment #4)
> Comment on attachment 430386 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=430386&action=review
> 
> > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb.in:267
> > +#if __IPHONE_OS_VERSION_MIN_REQUIRED <= 150000
> 
> 1) `<= a major version` is highly unusual and likely to lead to errors in
> the very near future. Does this need a FIXME + radar about reenabling it for
> future point updates?
> 2) what about the iOS-derived platforms that use this sandbox file but do
> not use __IPHONE_OS_VERSION_MIN_REQUIRED?

I have uploaded a new patch which I think should resolve these issues.

Thanks for reviewing!
Comment 7 Tim Horton 2021-06-02 12:41:38 PDT
Comment on attachment 430391 [details]
Patch

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

> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb.in:267
> +#if PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED < 150500

The new version check is odd for a different but very similar reason :) Maybe you meant < 160000?

Also, you definitely still need a FIXME.

Also, is it really OK that you're disabling this security feature for watchOS and tvOS?
Comment 8 Tim Horton 2021-06-02 12:55:25 PDT
Comment on attachment 430391 [details]
Patch

Per Arne explained both parts elsewhere, I retract my three comments :)
Comment 9 Per Arne Vollan 2021-06-02 13:00:20 PDT
Comment on attachment 430391 [details]
Patch

Thanks for reviewing!
Comment 10 EWS 2021-06-02 13:34:29 PDT
Committed r278370 (238397@main): <https://commits.webkit.org/238397@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 430391 [details].