[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
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.
