Drop-down Control borders missing

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Per Arne Vollan]

This can be seen when filing a bug on bugs.webkit.org.

[Per Arne Vollan]

<rdar://problem/40093461>

[Per Arne Vollan]

Created attachment 340058 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 340058 [details]
Patch

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

> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:643
> +(allow file-read-data (path-regex #"^/private/tmp/cv"))

This looks super suspicious. Any process on the system can modify WebKit behavior by writing to this location? That can’t be right, and I don’t think that it’s ok to allow.

[Per Arne Vollan]

(In reply to Alexey Proskuryakov from comment #3)
> Comment on attachment 340058 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=340058&action=review
> 
> > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:643
> > +(allow file-read-data (path-regex #"^/private/tmp/cv"))
> 
> This looks super suspicious. Any process on the system can modify WebKit
> behavior by writing to this location? That can’t be right, and I don’t think
> that it’s ok to allow.

This is intended as a temporary workaround until the underlying issue is fixed, but I do see your concern.

Thanks for reviewing!

[Brent Fulgham]

(In reply to Per Arne Vollan from comment #4)
> (In reply to Alexey Proskuryakov from comment #3)
> > Comment on attachment 340058 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=340058&action=review
> > 
> > > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:643
> > > +(allow file-read-data (path-regex #"^/private/tmp/cv"))
> > 
> > This looks super suspicious. Any process on the system can modify WebKit
> > behavior by writing to this location? That can’t be right, and I don’t think
> > that it’s ok to allow.
> 
> This is intended as a temporary workaround until the underlying issue is
> fixed, but I do see your concern.
> 
> Thanks for reviewing!

Yes, I agree with Alexey. I thought this sounded fine as a quick fix to allow someone to have proper function in a local build, but I don't think it's appropriate to land for general use.

We need to help find and fix the underlying regression that is causing this widget drawing weirdness.

[Per Arne Vollan]

(In reply to Brent Fulgham from comment #5)
> (In reply to Per Arne Vollan from comment #4)
> > (In reply to Alexey Proskuryakov from comment #3)
> > > Comment on attachment 340058 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=340058&action=review
> > > 
> > > > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:643
> > > > +(allow file-read-data (path-regex #"^/private/tmp/cv"))
> > > 
> > > This looks super suspicious. Any process on the system can modify WebKit
> > > behavior by writing to this location? That can’t be right, and I don’t think
> > > that it’s ok to allow.
> > 
> > This is intended as a temporary workaround until the underlying issue is
> > fixed, but I do see your concern.
> > 
> > Thanks for reviewing!
> 
> Yes, I agree with Alexey. I thought this sounded fine as a quick fix to
> allow someone to have proper function in a local build, but I don't think
> it's appropriate to land for general use.
> 
> We need to help find and fix the underlying regression that is causing this
> widget drawing weirdness.

Sounds good, thanks for reviewing!

[Daniel Bates]

Comment on attachment 340058 [details]
Patch

r-‘ing per comment 3 and comment 5 and to get this patch out of the review and cq queues.

[Per Arne Vollan]

Created attachment 340097 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 340097 [details]
Patch

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

> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:112
> +    (allow file-read* file-write-unlink
> +        (extension "com.apple.cvms.kernel"))

This seems to mean that a process with com.apple.cvms.kernel extension can read any files, and delete any files.

> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:114
> +    (allow file-read* file-write-unlink
> +        (prefix "/private/tmp/cvmsCodeSignObj"))

Doesn't this have the same problem as the original patch?

[Brent Fulgham]

Comment on attachment 340097 [details]
Patch

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

>> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:114
>> +        (prefix "/private/tmp/cvmsCodeSignObj"))
> 
> Doesn't this have the same problem as the original patch?

Per Arne: Can you try this instead:

(with-filter (extension "com.apple.cvms.kernel")
    (allow file-read* file-write-unlink
        (prefix "/private/tmp/cvmsCodeSignObj”)))

[Per Arne Vollan]

Created attachment 340111 [details]
Patch

[Brent Fulgham]

Comment on attachment 340111 [details]
Patch

This version looks good, based on what we discussed with the sandboxing team. r=me

[Per Arne Vollan]

Comment on attachment 340111 [details]
Patch

Thanks for reviewing, all!

[WebKit Commit Bot]

Comment on attachment 340111 [details]
Patch

Clearing flags on attachment: 340111

Committed r231653: <https://trac.webkit.org/changeset/231653>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.