Bug 221610

Summary: [macOS] Deny mach-lookup to the fonts service
Product: WebKit Reporter: Per Arne Vollan <pvollan>
Component: WebKit Misc.Assignee: Per Arne Vollan <pvollan>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, bfulgham, cdumez, cmarcelo, ews-watchlist, mmaxfield, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
bfulgham: review+
Patch none

Per Arne Vollan
Reported 2021-02-09 08:47:17 PST
Deny mach-lookup to the fonts service in the WebContent process on macOS.
Attachments
Patch (2.23 KB, patch)
2021-02-09 08:50 PST, Per Arne Vollan 		no flags
DetailsFormatted DiffDiff
Patch (11.18 KB, patch)
2021-02-16 06:21 PST, Per Arne Vollan 		bfulgham: review+
DetailsFormatted DiffDiff
Patch (11.35 KB, patch)
2021-02-17 07:21 PST, Per Arne Vollan 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Per Arne Vollan
Comment 1 2021-02-09 08:47:40 PST
<rdar://problem/69168609>
Per Arne Vollan
Comment 2 2021-02-09 08:50:21 PST
Created attachment 419727 [details] Patch
Brent Fulgham
Comment 3 2021-02-09 09:54:56 PST
Comment on attachment 419727 [details] Patch r=me
Per Arne Vollan
Comment 4 2021-02-09 10:00:27 PST
Comment on attachment 419727 [details] Patch Thanks for reviewing!
EWS
Comment 5 2021-02-09 10:03:56 PST
Committed r272586: <https://commits.webkit.org/r272586> All reviewed patches have been landed. Closing bug and clearing flags on attachment 419727 [details].
Myles C. Maxfield
Comment 6 2021-02-10 08:46:09 PST
Comment on attachment 419727 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419727&action=review > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:1114 > #if __MAC_OS_X_VERSION_MIN_REQUIRED <= 110000 shouldn't this be "< 120000"?
Per Arne Vollan
Comment 7 2021-02-10 12:17:59 PST
(In reply to Myles C. Maxfield from comment #6) > Comment on attachment 419727 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=419727&action=review > > > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:1114 > > #if __MAC_OS_X_VERSION_MIN_REQUIRED <= 110000 > > shouldn't this be "< 120000"? That is a good point, although in this case, I think these should also be blocked when OS version > 11. Thanks for reviewing!
Per Arne Vollan
Comment 8 2021-02-10 12:20:04 PST
Reverted in <https://trac.webkit.org/changeset/272657/webkit>
Per Arne Vollan
Comment 9 2021-02-16 06:21:41 PST
Created attachment 420465 [details] Patch
Brent Fulgham
Comment 10 2021-02-16 10:04:35 PST
Comment on attachment 420465 [details] Patch Nice! r=me
Per Arne Vollan
Comment 11 2021-02-17 07:21:11 PST
Created attachment 420641 [details] Patch
Per Arne Vollan
Comment 12 2021-02-17 07:22:30 PST
Comment on attachment 420641 [details] Patch Thanks for reviewing!
EWS
Comment 13 2021-02-17 08:16:26 PST
Committed r272999: <https://commits.webkit.org/r272999> All reviewed patches have been landed. Closing bug and clearing flags on attachment 420641 [details].
Myles C. Maxfield
Comment 14 2021-03-10 17:15:50 PST
Comment on attachment 420641 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420641&action=review > Source/WebKit/UIProcess/WebPageProxy.cpp:7692 > +static bool disableStaticFontRegistry() "disable" is a verb, but the implementation of this function appears to not actually disable anything.
Chris Dumez
Comment 15 2021-03-10 17:25:18 PST
Comment on attachment 420641 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420641&action=review > Source/WebKit/UIProcess/WebPageProxy.cpp:7686 > + if (String(static_cast<CFStringRef>(font)) != "System") Seems needlessly expensive to construct a WTF::String here: if (CFStringCompare(font, CFSTR("System"), kCFCompareCaseSensitive) != kCFCompareEqualTo)
Note You need to log in before you can comment on or make changes to this bug.