|
Message-ID: <CABg4E-mTMVfxB8t7zOX+00Gu24pxazxPOdJ691DpaDRa0V-mLA@mail.gmail.com> Date: Mon, 8 Jul 2019 22:17:29 -0400 From: Tavian Barnes <tavianator@...ianator.com> To: musl@...ts.openwall.com Subject: Re: posix_spawn() can expose the error pipe to the spawned process [Sorry for messing up the threading, I'm not subscribed so I grabbed this from the archive] On Mon, 8 Jul 2019 at 13:09:55 -0400, Rich Felker wrote: > 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. Agreed. > Does the attached fix look ok to you? Yep, looks right. I was going to suggest moving the check next to the if (op->fd == p) check above, but it looks like op->srcfd isn't always initialized. > 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. Yep, agreed. I don't think the current behaviour is a bug or non-standard-compliant or anything. Just seemed like an easy QoI improvement. > Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.