posix_spawn() can expose the error pipe to the spawned process

Submitted by Rich Felker on July 8, 2019, 5:09 p.m.

Details

Message ID 20190708170955.GJ1506@brightrain.aerifal.cx
State New
Series "posix_spawn() can expose the error pipe to the spawned process"
Headers show

Commit Message

Rich Felker July 8, 2019, 5:09 p.m.
On Mon, Jul 08, 2019 at 11:39:49AM -0400, Tavian Barnes wrote:
> posix_spawn[p]() is implemented with a pipe that sends any error codes
> encountered back to the parent process.  It attempts to move the pipe
> out of the way with dup() whenever that fd is used by the file_actions
> as an output, but not as an input.  So something like this:
> 
> $ cat spawn_pipe.c
> #include <spawn.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> 
> extern char **environ;
> 
> int main() {
>         posix_spawn_file_actions_t fa;
>         posix_spawn_file_actions_init(&fa);
>         posix_spawn_file_actions_adddup2(&fa, 4, 1);
> 
>         char *argv[] = { "printf", "\\5\\0\\0\\0", NULL };
> 
>         pid_t pid;
>         int ret = posix_spawnp(&pid, "printf", &fa, NULL, argv, environ);
>         fprintf(stderr, "posix_spawnp(): %s\n", strerror(ret));
>         return ret;
> }
> $ musl-gcc -Wall spawn_pipe.c -o spawn_pipe && ./spawn_pipe
> posix_spawnp(): I/O error
> 
> ends up writing to that pipe and causing posix_spawn() to report
> arbitrary errors.  Presumably it should fail before exec()ing with
> EBADF instead.

Thanks! To clarify, for anyone reading, the issue here is that you're
able to use a dup2 action in the spawn file actions to copy, and
thereby obtain the ability to send junk to, the pipe file descriptor
used internally. It's expected that the implementation can use file
descriptors internally, and that if you use/copy fds you don't own,
you could end up accessing one of them (this is the rationale for why
POSIX has no closeall operation). However it seems preferable to avoid
getting into an internally inconsistent state if this happens, and
that should be easy to do.

Does the attached fix look ok to you?

Note that there are still plenty of other ways you can do evil things
by copying internal fds, e.g. racing with another thread also calling
posix_spawn to copy its pipe fd, or anywhere else fds are used
(locale, message catalog, timezone, etc. loading, hosts/dns lookups,
...). These are pretty much fundamental issues in using dup2 with a fd
you don't own.

Rich

Patch hide | download patch | download mbox

diff --git a/src/process/posix_spawn.c b/src/process/posix_spawn.c
index 5aaf829d..306faa05 100644
--- a/src/process/posix_spawn.c
+++ b/src/process/posix_spawn.c
@@ -101,6 +101,10 @@  static int child(void *args_vp)
 				break;
 			case FDOP_DUP2:
 				fd = op->srcfd;
+				if (fd == p) {
+					ret = -EBADF;
+					goto fail;
+				}
 				if (fd != op->fd) {
 					if ((ret=__sys_dup2(fd, op->fd))<0)
 						goto fail;