Some ideas to make mitosis secure

[alecmocatta]

Hey @Manishearth, I have some thoughts on how to make the serialization and deserialization of closures more secure. I'm sure it's not a realistic problem for the current users/uses of mitosis, but it is currently vulnerable to arbitrary code execution due to the way closures are deserialized. A hypothetical example: a binary using mitosis with its SUID bit set and owned by root can be used for privilege escalation by non-root users¹.

I've been tackling a related problem – how to send closures between processes over the network – and have taken a similar but slightly different approach². I have an idea for how to achieve soundness, and I'd be keen to hear your thoughts.

I see you are casting non-capturing closures to function pointers, made relative to a known base. I took this approach at first, but I could not see an attractive route to making this sound so I switched to using something that rustc has more control over: trait objects.

Essentially my idea for soundness is an addition to Rust that enables programs to get all vtables for a particular trait. I have a working PR for that here rust-lang/rust#66113. Given that addition, on serialization of a closure upcast to a trait object you serialize the concrete type_id then the data³. On deserialization you look up the type_id to find the vtable, and if found then call the vtable method to deserialize the data. (See the PR for more details.) This is⁴ a way to achieve secure & sound deserialization of trait objects. There is some more research including prior art in a languishing RFC I've worked on here.

I'd love my projects and mitosis to be able to deserialize closures securely so I'm keen to hear if you have any thoughts as to how best to go about achieving this.

---

1. by creating an IPC channel, running the mitosis-using binary with MITOSIS_CONTENT_PROCESS_ID set to the channel token, and then sending along the channel the arbitrary address to execute.
2. if you're interested here are the various related crates I've published:
   • serde_closure – macros that wrap closures to make them (including captured variables) safely serializable and deserializable.
   • serde_traitobject – supertraits that enable the serialization and deserialization of trait objects. It is currently, like mitosis, not sound or secure – though it does some validation to rule out non-malicious errors (see here) before dereferencing the received pointer.
   • constellation – uses the above to spawn closures across a cluster (standalone or kubernetes).
3. the closure (including its captured variables) can made serializable and deserializable via for example serde_closure.
4. once Collisions in type_id rust-lang/rust#10389 is fixed.

[Manishearth]

I haven't really thought much about this, I do feel that you should probably not suid processes without cleaning env vars.

@oli-cosmian may have opinions

[oli-cosmian]

I don't have the time to properly read up on the suggested proposal and all its ramifications. I did read the linked rustc PR though.

If the feature is accepted and implemented, it seems like it would be possible to make mitosis use that instead of the current scheme. The feature would have to be stable though before mitosis could use it.

On the main point of this issue (mitosis being unsound, and very bad with setuid root): I feel this is unsound on the same level that writing to /proc/self/mem is unsound out of two reasons:

1. setting the SUID bit for a binary should only be done if you have checked whether the program can't execute arbitrary code. If you have a binary that uses std::process::Command to run whatever you pass in some env var or argument, then it's equally bad to make that binary setuid root, just like doing that to a mitosis using program.
2. the env var used by mitosis is an internal detail that is not part of the public API, setting it and invoking a Rust binary is no different from invoking a non-Rust binary. Rust only guarantees safety within its language, and invoking the mitosis crate will keep these safety guarantees, because the newly spawned process is not part of the Rust program and will never cause UB in the Rust program.

You can always screw with mitosis by running inside a chroot (thus hiding /proc/exe, causing the fallback logic to invoke std::env::current_exe()) and overwriting the binary with another binary before running mitosis::spawn. Again, this is not unsound as far as the Rust language is concerned, because all the Bad Things are happening in another process.