1858272 - [forkserver] Send arguments and environment directly to the forked child process

Status: RESOLVED FIXED

(Core :: IPC, task)

Description

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

One of the remaining issues with the fork server is that these attempts to avoid leaking one processes' parameters into subsequent child processes may not work due to compiler optimizations and/or other unexpected copies of the data left on the heap. At present I don't think there's anything sensitive that could be leaked that way (i.e., secrets that could allow impersonating another process / privilege escalation / sandbox escape) which the attacker wouldn't already legitimately have, and mozjemalloc heap poisoning may reduce the possibility of exploitation, but we should fix this.

The idea I have is to send the argv/env strings directly to the forked process after it's forked, so that the forkserver never sees them. Specifically, the message sent to the fork server will have an extra attached fd, which the forked process reads a second message from with the serialized argv/env.

This is related to bug 1752638, because the way I was planning to add additional messages to the forkserver protocol was to make the fork request an async message with an attached socket used for the reply. (I don't necessarily need to do this bug before that one, but it's probably easier that way.)

But also, in order to make this work, I'll need to clean up the sandbox integration (which I was going to do anyway) so that we're not using the environment to pass information that's needed before the clone call. That will be a separate bug.

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

Attachment: Bug 1858272 - Change fork server protocol to send args/env directly to child processes.

The argument and environment strings for a child process are now sent
directly to that process after it's forked, in a separate IPC message,
rather than to the fork server. The initial message sent to the fork
server carries an attached socket used only for that request, which is
used to construct an additional MiniTransceiver instance for this new
message.

This ensures that those strings aren't present in the fork server's
address space and can't accidentally leak into other child processes;
currently there's some code that attempts to overwrite all copies of the
data in memory but this could be defeated by compiler optimizations,
library code making extra copies while resizing buffers, or other
mistakes. It's safer to follow the principle of least privilege.

This patch also lays the groundwork for future changes to the fork
server protocol: if the per-request socket also carried the reply to
the fork request, the main channel could carry other messages in both
directions. (This patch does not do that, yet; the main protocol still
contains the equivalent of exactly one sync message type.)

Note that file descriptors to be passed to the child process are still
sent to the fork server rather than directly to the child; this isn't
ideal, but it's harmless for security (we can be relatively sure that
fds are closed as intended and there are no accidental duplicates) and
it made the patches less complicated (this one and also bug 1858279).

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/159b34aba2fd
Change fork server protocol to send args/env directly to child processes. r=ipc-reviewers,nika

Comment 3

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/159b34aba2fd