Bug 180307

Summary: IPC code doesn't understand NSDictionaries with non-NSString keys
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: WebKit2Assignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, andersca, ap, beidson, bfulgham, commit-queue, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=188008
Attachments:
Description Flags
Needs test
none
Patch none

Myles C. Maxfield
Reported 2017-12-01 22:50:16 PST
IPC code doesn't understand NSDictionaries with non-NSString keys
Attachments
Needs test (2.01 KB, patch)
2017-12-01 22:50 PST, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (4.50 KB, patch)
2017-12-12 13:58 PST, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Myles C. Maxfield
Comment 1 2017-12-01 22:50:35 PST
Created attachment 328214 [details] Needs test
Myles C. Maxfield
Comment 2 2017-12-01 22:51:23 PST
<rdar://problem/35812382>
Myles C. Maxfield
Comment 3 2017-12-01 22:52:43 PST
Is there a direct way to test these? Or do I have to use EventSender?
Alexey Proskuryakov
Comment 4 2017-12-02 11:42:29 PST
We usually send WTF objects over CoreIPC. Sending Foundation objects just has too many pitfalls, including correctness, performance and security. I recommend looking into removing the existing code here, not into extending it.
Brady Eidson
Comment 5 2017-12-02 15:14:08 PST
Without even looking at the patch, I agree with Alexey. Attempts to serialize Cocoa types are often wrought with problems and are not entirely within our control. Better to use WTF/WebCore/WebKit types when possible
Myles C. Maxfield
Comment 6 2017-12-04 09:26:35 PST
So it's better to convert to WTF types, send across the wire, then convert back to NS types, rather than just fixing a bug in our existing converters?
Alexey Proskuryakov
Comment 7 2017-12-04 10:23:09 PST
Yes. Or better yet, not use NS types in WebCore at all. The security aspect of it is not just about what you do, but also about what malicious code running in WebContent can do to escape the sandbox. Any deserialization of NS types performed in UI process adds a large attack surface.
Alexey Proskuryakov
Comment 8 2017-12-04 10:30:39 PST
Oh wait, this doesn't actually use Foundation serialization. What I said is mostly irrelevant, I should have studied the patch more closely. Freaked out because of the recent NSSecureCoding fixes.
Myles C. Maxfield
Comment 9 2017-12-12 13:58:34 PST
Created attachment 329151 [details] Patch
WebKit Commit Bot
Comment 10 2017-12-12 15:11:23 PST
Comment on attachment 329151 [details] Patch Clearing flags on attachment: 329151 Committed r225811: <https://trac.webkit.org/changeset/225811>
WebKit Commit Bot
Comment 11 2017-12-12 15:11:25 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.