Bug 234128

Summary: Revoke attachment element sandbox extensions
Product: WebKit Reporter: Per Arne Vollan <pvollan>
Component: WebKit Misc.Assignee: Per Arne Vollan <pvollan>
Status: NEW ---    
Severity: Normal CC: bfulgham, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
bfulgham: review+, ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch ews-feeder: commit-queue-

Description Per Arne Vollan 2021-12-10 00:44:16 PST 
Attachment element sandbox extensions should be revoked in order to block further use of these extensions.
Comment 1 Per Arne Vollan 2021-12-10 00:53:05 PST 
Created attachment 446676 [details]
Patch
Comment 2 Per Arne Vollan 2021-12-10 01:08:47 PST 
Created attachment 446679 [details]
Patch
Comment 3 Per Arne Vollan 2021-12-10 01:18:20 PST 
Created attachment 446680 [details]
Patch
Comment 4 Per Arne Vollan 2021-12-10 01:22:36 PST 
Created attachment 446681 [details]
Patch
Comment 5 Per Arne Vollan 2021-12-10 01:37:34 PST 
Created attachment 446683 [details]
Patch
Comment 6 Per Arne Vollan 2021-12-10 01:42:39 PST 
Created attachment 446684 [details]
Patch
Comment 7 Per Arne Vollan 2021-12-10 01:51:50 PST 
Created attachment 446686 [details]
Patch
Comment 8 Per Arne Vollan 2021-12-10 02:00:01 PST 
Created attachment 446688 [details]
Patch
Comment 9 Per Arne Vollan 2021-12-10 02:07:34 PST 
Created attachment 446689 [details]
Patch
Comment 10 Per Arne Vollan 2021-12-10 02:30:26 PST 
Created attachment 446695 [details]
Patch
Comment 11 Per Arne Vollan 2021-12-10 02:32:10 PST 
Created attachment 446696 [details]
Patch
Comment 12 Per Arne Vollan 2021-12-10 02:45:13 PST 
Created attachment 446698 [details]
Patch
Comment 13 Per Arne Vollan 2021-12-10 09:19:59 PST 
Created attachment 446742 [details]
Patch
Comment 14 Per Arne Vollan 2021-12-10 12:02:53 PST 
Created attachment 446776 [details]
Patch
Comment 15 Brent Fulgham 2021-12-10 14:37:17 PST 
Comment on attachment 446776 [details]
Patch

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

r=me, but please consider the naming and adding a comment to reduce confusion about this new method.

> Source/WebCore/PAL/pal/spi/cocoa/ISIconManagerSPI.h:28
> +@interface ISIconManager : NSObject

If this is defined in an internal header, shouldn't we do the usual "#if USE(APPLE_INTERNAL_SDK)" thing and grab it directly?

> Source/WebKit/ChangeLog:10
> +        ISIconManager.

Maybe we should name this "consumeAndRevokeAttachmentElementSandboxExtensions" or something, and add a comment in it that the extensions are not needed after the ISIconManager is initialized?
Comment 16 Radar WebKit Bug Importer 2021-12-17 00:45:19 PST 
<rdar://problem/86619872>
Comment 17 Per Arne Vollan 2022-01-29 19:01:19 PST 
Created attachment 450344 [details]
Patch
Comment 18 Per Arne Vollan 2022-01-29 19:03:52 PST 
Created attachment 450345 [details]
Patch
Comment 19 Per Arne Vollan 2022-01-29 19:05:21 PST 
Created attachment 450346 [details]
Patch
Comment 20 Per Arne Vollan 2022-01-29 19:06:08 PST 
(In reply to Brent Fulgham from comment #15)
> Comment on attachment 446776 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=446776&action=review
> 
> r=me, but please consider the naming and adding a comment to reduce
> confusion about this new method.
> 
> > Source/WebCore/PAL/pal/spi/cocoa/ISIconManagerSPI.h:28
> > +@interface ISIconManager : NSObject
> 
> If this is defined in an internal header, shouldn't we do the usual "#if
> USE(APPLE_INTERNAL_SDK)" thing and grab it directly?
> 

That's a good point. I will address that in an upcoming patch.

> > Source/WebKit/ChangeLog:10
> > +        ISIconManager.
> 
> Maybe we should name this
> "consumeAndRevokeAttachmentElementSandboxExtensions" or something, and add a
> comment in it that the extensions are not needed after the ISIconManager is
> initialized?

Fixed!

Thanks for reviewing!