Investigate moving away from DO (Distributed Object) as IPC mechanism #1157
Comments
Open
6 tasks
ychin
added a commit
that referenced
this issue
Sep 12, 2023
Currently, MacVim uses Distributed Objects / NSConnection as the IPC mechanism. The child Vim process connects to the parent MacVim process using NSConnection and registers itself. A security issue with this is that NSConnection is global, and any process can connect to the app, and MacVim isn't too hardened against this issue. Note that one reason why we do need the ability for the MacVim app to accept random connections is to support the `:gui` command from a random Vim process, and to supported listing server names. One issue is that while the app protocol (MMAppProtocol) is only a few functions, we were exposing the entire app, which exposes functions like `executeInLoginShell`, which could be invoked by the caller, which is quite unsafe as it could be invoked by any third-party app. Fix this issue by using `NSProtocolChecker` to make sure we only expose the APIs that we want to expose. Each Vim controller now also gets a randomized ID instead of an incremental one. Currently the API for sending messages from Vim to MacVim is public, meaning any app can send message to MacVim. Using a randomized ID makes it more difficult for an attacker to guess the ID (which used to always start at 1) and injects random commands to MacVim pretending to be the Vim process. Also, make sure if MacVim failed to register the NSConnection on launch, just display an error and terminate. This usually happens if multiple MacVim instances are opened, but also if an attacking app is trying to register a connection first using the same name. This way the user would know something is wrong instead of MacVim being opened by not able to do anything as it didn't register the connection. In the near future, the IPC mechanism will be switched to XPC, which is the preferred way by Apple as Distributed Objects has been deprecated for a long time. It will have proper security to only accept processes within the same app to message each other. It will be done in #1157.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
MacVim currently uses Distributed Object as a way to communicate between individual Vim processes and MacVim but it's long been deprecated by Apple. See https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/DistrObjects/DistrObjects.html. As such, all usages of
NSConnectionare currently throwing deprecation warning.Instead, Apple's preferred APIs are the XPC system: https://developer.apple.com/documentation/xpc
We should investigate whether to move off DO to either Apple's XPC or another IPC mechanism (for example, Neovim has its own IPC mechanism via msgpack). Since the DO implementation is the underlying system that we talks between MacVim GUI and Vim process, and there are a few quirks and back-and-forth messaging, we should make sure it keeps working (some edge cases includes window resizing with/without guioption-k, fullscreen, input, etc), and latency won't be affected as a result.
Also, occasionally we see error messages with DO (#1005), and I have seen issues with running unit tests locally if I have certain settings that seem to stem from DO init race connection, so hopefully we can eliminate those as well.
The text was updated successfully, but these errors were encountered: