[webkit-reviews] review granted: [Bug 218188] [macOS] Avoid calling getpwuid_r before entering the sandbox in the WebContent process : [Attachment 420863] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Per Arne Vollan
<pvollan at apple.com>'s request for review:
Bug 218188: [macOS] Avoid calling getpwuid_r before entering the sandbox in the
WebContent process
https://bugs.webkit.org/show_bug.cgi?id=218188

Attachment 420863: Patch

https://bugs.webkit.org/attachment.cgi?id=420863&action=review




--- Comment #15 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 420863
  --> https://bugs.webkit.org/attachment.cgi?id=420863
Patch

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

>>> Source/WebKit/ChangeLog:9
>>> +	     opendirectoryd, which should be avoided. Instead, call getpwuid_r
in the UI process, and pass the results to the
>> 
>> This is the second case I've looked at today where we left open a privileged
connection in WebContent. Is there a way to write a test for these kinds of
open connections? I'm not sure how we'll defend against regression without a
test.
> 
> Yes, that is a very good point. I think it should be possible to write
regression tests for this, although it is probably not trivial, since the
sandbox API cannot detect this, AFAIK. I would think there exists API for
listing open mach ports, though. Additionally, I think we should look into
entering the sandbox much earlier, which would also help defend against this.
> 
> Thanks for reviewing!

Perhaps you could file a bug to create this new test feature?

> Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:687
> +    if (homeDirectory.isEmpty()) {

When might we ever need to his this code path? I guess third-party clients
might not send the home directory?

> Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:110
> +    struct passwd* result = 0;

nullptr