Bug 197844

Summary: Correct the sandbox to allow loading libraries from /Library/Apple
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit2Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, bfulgham, bshafiei, commit-queue, pvollan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Brent Fulgham 2019-05-13 10:11:12 PDT
Some InjectedBundles need to load libraries from "/Library/Apple", which is not allowed by default. Revise the sandbox to support this additional location for frameworks.
Comment 1 Brent Fulgham 2019-05-13 10:13:11 PDT
Created attachment 369744 [details]
Patch
Comment 2 Brent Fulgham 2019-05-13 10:17:17 PDT
<rdar://problem/50727815>
Comment 3 Per Arne Vollan 2019-05-13 10:27:46 PDT
Comment on attachment 369744 [details]
Patch

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

R=me.

> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:43
>  (allow file-read*
>      (require-all (file-mode #o0004)
>      (require-any (subpath "/Library/Filesystems/NetFSPlugins")
> +    (subpath "/Library/Apple/System")

Is this only needed for the injected bundle? If that is the case, maybe we could issue an extension? I don't think this is required now, and could be done in a followup patch.

> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:63
> +;;; Allow mapping of system frameworks + dylibs
> +(allow file-map-executable
> +    (subpath "/Library/Apple/System/Library/Frameworks")
> +    (subpath "/Library/Apple/System/Library/PrivateFrameworks")
> +    (subpath "/System/Library/Frameworks")
> +    (subpath "/System/Library/PrivateFrameworks")
> +    (subpath "/usr/lib")

Ditto.
Comment 4 Brent Fulgham 2019-05-13 10:44:47 PDT
Comment on attachment 369744 [details]
Patch

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

>> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:43
>> +    (subpath "/Library/Apple/System")
> 
> Is this only needed for the injected bundle? If that is the case, maybe we could issue an extension? I don't think this is required now, and could be done in a followup patch.

After reviewing things further, I think we should just treat this as another canonical location where system frameworks might live. It's not specific to injected bundles.

>> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:63
>> +    (subpath "/usr/lib")
> 
> Ditto.

This section might not be needed (yet), because we don't prohibit file mapping. But having this here will allow us to flip on that protection in a future patch.
Comment 5 WebKit Commit Bot 2019-05-13 12:53:50 PDT
Comment on attachment 369744 [details]
Patch

Clearing flags on attachment: 369744

Committed r245246: <https://trac.webkit.org/changeset/245246>
Comment 6 WebKit Commit Bot 2019-05-13 12:53:51 PDT
All reviewed patches have been landed.  Closing bug.