Summary: | [Mac] Use the PID of the WebContent process when issuing local file read sandbox extensions | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Per Arne Vollan <pvollan> | ||||||||||
Component: | WebKit Misc. | Assignee: | Per Arne Vollan <pvollan> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | benjamin, bfulgham, cdumez, cmarcelo, commit-queue, dbates, ews-watchlist, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 200772, 202012 | ||||||||||||
Bug Blocks: | 206544 | ||||||||||||
Attachments: |
|
Description
Per Arne Vollan
2019-08-08 11:23:01 PDT
Created attachment 375829 [details]
Patch
Comment on attachment 375829 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375829&action=review Looks good. > Source/WebKit/ChangeLog:9 > + the WebContent process. Maybe it would be better phrased as: "Adopt SPI to issue a process-specific sandbox extension for local file read, passing it the process identifier of the WebContent process." (In reply to Brent Fulgham from comment #3) > Comment on attachment 375829 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=375829&action=review > > Looks good. > > > Source/WebKit/ChangeLog:9 > > + the WebContent process. > > Maybe it would be better phrased as: > > "Adopt SPI to issue a process-specific sandbox extension for local file > read, passing it the process identifier of the WebContent process." Thanks for reviewing! I will update the patch. Created attachment 375832 [details]
Patch
Comment on attachment 375832 [details] Patch Clearing flags on attachment: 375832 Committed r248440: <https://trac.webkit.org/changeset/248440> Created attachment 376628 [details]
Patch
Comment on attachment 376628 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=376628&action=review I think the log message should be changed, but otherwise this looks good. R=me. > Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm:357 > + WTFLogAlways("Could not create a '%s' sandbox extension", path.utf8().data()); Thislog message could leak potentially private user data to our logs. Could you change it to one of the LOG_DEBUG macros, or perhaps just remove the path from the log file? it should be enough to log that a file extension could not be generated. Created attachment 376647 [details]
Patch
(In reply to Brent Fulgham from comment #8) > Comment on attachment 376628 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=376628&action=review > > I think the log message should be changed, but otherwise this looks good. > R=me. > > > Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm:357 > > + WTFLogAlways("Could not create a '%s' sandbox extension", path.utf8().data()); > > Thislog message could leak potentially private user data to our logs. Could > you change it to one of the LOG_DEBUG macros, or perhaps just remove the > path from the log file? it should be enough to log that a file extension > could not be generated. Done. Thanks for reviewing! Comment on attachment 376647 [details] Patch Clearing flags on attachment: 376647 Committed r248832: <https://trac.webkit.org/changeset/248832> |