Bug 180307 - IPC code doesn't understand NSDictionaries with non-NSString keys
Summary: IPC code doesn't understand NSDictionaries with non-NSString keys
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
Keywords: InRadar
Depends on:
Reported: 2017-12-01 22:50 PST by Myles C. Maxfield
Modified: 2018-09-10 19:56 PDT (History)
7 users (show)

See Also:

Needs test (2.01 KB, patch)
2017-12-01 22:50 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (4.50 KB, patch)
2017-12-12 13:58 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2017-12-01 22:50:16 PST
IPC code doesn't understand NSDictionaries with non-NSString keys
Comment 1 Myles C. Maxfield 2017-12-01 22:50:35 PST
Created attachment 328214 [details]
Needs test
Comment 2 Myles C. Maxfield 2017-12-01 22:51:23 PST
Comment 3 Myles C. Maxfield 2017-12-01 22:52:43 PST
Is there a direct way to test these? Or do I have to use EventSender?
Comment 4 Alexey Proskuryakov 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.
Comment 5 Brady Eidson 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
Comment 6 Myles C. Maxfield 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?
Comment 7 Alexey Proskuryakov 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.
Comment 8 Alexey Proskuryakov 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.
Comment 9 Myles C. Maxfield 2017-12-12 13:58:34 PST
Created attachment 329151 [details]
Comment 10 WebKit Commit Bot 2017-12-12 15:11:23 PST
Comment on attachment 329151 [details]

Clearing flags on attachment: 329151

Committed r225811: <https://trac.webkit.org/changeset/225811>
Comment 11 WebKit Commit Bot 2017-12-12 15:11:25 PST
All reviewed patches have been landed.  Closing bug.