Bug 234128 - Revoke attachment element sandbox extensions
Summary: Revoke attachment element sandbox extensions
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-12-10 00:44 PST by Per Arne Vollan
Modified: 2022-01-29 19:57 PST (History)
2 users (show)

See Also:

Attachments
Patch (14.24 KB, patch)
2021-12-10 00:53 PST, Per Arne Vollan 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (14.22 KB, patch)
2021-12-10 01:08 PST, Per Arne Vollan 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (14.23 KB, patch)
2021-12-10 01:18 PST, Per Arne Vollan 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (14.24 KB, patch)
2021-12-10 01:22 PST, Per Arne Vollan 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (14.25 KB, patch)
2021-12-10 01:37 PST, Per Arne Vollan 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (14.25 KB, patch)
2021-12-10 01:42 PST, Per Arne Vollan 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (14.26 KB, patch)
2021-12-10 01:51 PST, Per Arne Vollan 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (14.26 KB, patch)
2021-12-10 02:00 PST, Per Arne Vollan 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (14.26 KB, patch)
2021-12-10 02:07 PST, Per Arne Vollan 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (14.24 KB, patch)
2021-12-10 02:30 PST, Per Arne Vollan 		no flags Details | Formatted Diff | Diff
Patch (14.66 KB, patch)
2021-12-10 02:32 PST, Per Arne Vollan 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (14.70 KB, patch)
2021-12-10 02:45 PST, Per Arne Vollan 		no flags Details | Formatted Diff | Diff
Patch (14.85 KB, patch)
2021-12-10 09:19 PST, Per Arne Vollan 		no flags Details | Formatted Diff | Diff
Patch (15.39 KB, patch)
2021-12-10 12:02 PST, Per Arne Vollan 		bfulgham: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (15.70 KB, patch)
2022-01-29 19:01 PST, Per Arne Vollan 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (15.73 KB, patch)
2022-01-29 19:03 PST, Per Arne Vollan 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (15.81 KB, patch)
2022-01-29 19:05 PST, Per Arne Vollan 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (15) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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!