WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
210937
[iOS] Fix sandbox violation when uploading a file
https://bugs.webkit.org/show_bug.cgi?id=210937
Summary
[iOS] Fix sandbox violation when uploading a file
Per Arne Vollan
Reported
2020-04-23 14:53:05 PDT
On iOS, the file chooser needs access to frontboard and icon services in the WebContent process. Create and consume extensions for these services when choosing files. When done, the extensions should be revoked.
Attachments
Patch
(12.79 KB, patch)
2020-04-24 12:29 PDT
,
Per Arne Vollan
no flags
Details
Formatted Diff
Diff
Patch
(12.80 KB, patch)
2020-04-24 12:32 PDT
,
Per Arne Vollan
no flags
Details
Formatted Diff
Diff
Patch
(12.04 KB, patch)
2020-04-24 13:30 PDT
,
Per Arne Vollan
no flags
Details
Formatted Diff
Diff
Patch
(12.19 KB, patch)
2020-04-24 14:41 PDT
,
Per Arne Vollan
darin
: review+
Details
Formatted Diff
Diff
Patch
(18.81 KB, patch)
2020-04-27 14:00 PDT
,
Per Arne Vollan
no flags
Details
Formatted Diff
Diff
Patch
(19.22 KB, patch)
2020-04-27 14:35 PDT
,
Per Arne Vollan
no flags
Details
Formatted Diff
Diff
Patch
(19.58 KB, patch)
2020-04-27 15:01 PDT
,
Per Arne Vollan
no flags
Details
Formatted Diff
Diff
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
Per Arne Vollan
Comment 1
2020-04-24 12:29:26 PDT
Created
attachment 397484
[details]
Patch
Per Arne Vollan
Comment 2
2020-04-24 12:32:27 PDT
Created
attachment 397486
[details]
Patch
Per Arne Vollan
Comment 3
2020-04-24 13:30:29 PDT
Created
attachment 397497
[details]
Patch
Per Arne Vollan
Comment 4
2020-04-24 14:41:28 PDT
Created
attachment 397511
[details]
Patch
Darin Adler
Comment 5
2020-04-24 18:04:57 PDT
Comment on
attachment 397511
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=397511&action=review
> Source/WebKit/Shared/ios/WebIconUtilities.mm:95 > + if (![interactionController.icons count]) > + return nullptr;
How does this change relate to the rest? No comment in the change log to explain it. Definitely not obvious. Do we have to carefully call this before the extension is revoked? Also, UIImage * return value would normally be a nil, not *. Also, looks like there is a mistake in thumbnailSizedImageForImage, which should return a RetainPtr<UIImage> or should retain/autorelease the result of UIGraphicsGetImageFromCurrentImageContext. Given that name it doesn’t sound safe to keep the pointer around before deallocating the image context.
Per Arne Vollan
Comment 6
2020-04-27 14:00:20 PDT
Created
attachment 397732
[details]
Patch
Per Arne Vollan
Comment 7
2020-04-27 14:08:17 PDT
(In reply to Darin Adler from
comment #5
)
> Comment on
attachment 397511
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=397511&action=review
> > > Source/WebKit/Shared/ios/WebIconUtilities.mm:95 > > + if (![interactionController.icons count]) > > + return nullptr; > > How does this change relate to the rest? No comment in the change log to > explain it. Definitely not obvious. >
When running the test associated with this patch in the iOS simulator, I encountered an out-of-bounds array exception, which was fixed by the change above. I added a comment about this in the change log.
> Do we have to carefully call this before the extension is revoked? > > Also, UIImage * return value would normally be a nil, not *. >
Fixed.
> Also, looks like there is a mistake in thumbnailSizedImageForImage, which > should return a RetainPtr<UIImage> or should retain/autorelease the result > of UIGraphicsGetImageFromCurrentImageContext. Given that name it doesn’t > sound safe to keep the pointer around before deallocating the image context.
I changed the return type to RetainPtr<UIImage> in the most recent patch. This lead to a few other changes. I did not put the bug back into the Review state, but it would perhaps be good if you could have another quick look to make sure the changes look good to you. Thanks for reviewing!
Per Arne Vollan
Comment 8
2020-04-27 14:35:25 PDT
Created
attachment 397738
[details]
Patch
Darin Adler
Comment 9
2020-04-27 14:48:51 PDT
Comment on
attachment 397738
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=397738&action=review
> Source/WebKit/Shared/ios/WebIconUtilities.mm:86 > return resultImage;
Need to change the resultImage local variable to a RetainPtr. If UIGraphicsEndImageContext released then image, then this return statement is too late.
Per Arne Vollan
Comment 10
2020-04-27 15:01:02 PDT
Created
attachment 397749
[details]
Patch
Per Arne Vollan
Comment 11
2020-04-27 15:01:31 PDT
(In reply to Darin Adler from
comment #9
)
> Comment on
attachment 397738
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=397738&action=review
> > > Source/WebKit/Shared/ios/WebIconUtilities.mm:86 > > return resultImage; > > Need to change the resultImage local variable to a RetainPtr. If > UIGraphicsEndImageContext released then image, then this return statement is > too late.
Good catch, thanks for reviewing!
EWS
Comment 12
2020-04-28 06:09:26 PDT
Committed
r260820
: <
https://trac.webkit.org/changeset/260820
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 397749
[details]
.
Radar WebKit Bug Importer
Comment 13
2020-04-28 06:10:17 PDT
<
rdar://problem/62513703
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug