From: Eric Blake Date: Wed, 19 Aug 2009 16:02:19 +0000 (-0600) Subject: popen-safer: prevent popen from clobbering std descriptors X-Git-Tag: v0.1~5637 X-Git-Url: http://erislabs.org.uk/gitweb/?a=commitdiff_plain;h=ccb9c6798066958dd14c1005a7d5ffc1cbd6edae;p=gnulib.git popen-safer: prevent popen from clobbering std descriptors * modules/popen-safer: New file. * lib/popen-safer.c: Likewise. * m4/stdio-safer.m4 (gl_POPEN_SAFER): New macro. * lib/stdio--.h (popen): Provide override. * lib/stdio-safer.h (popen_safer): Provide declaration. * tests/test-popen.c (includes): Partially test this. * modules/popen-safer-tests: New file, for more tests. * tests/test-popen-safer.c: Likewise. * MODULES.html.sh (file stream based Input/Output): Mention it. Signed-off-by: Eric Blake --- diff --git a/ChangeLog b/ChangeLog index b5d16e644..2161c2b0b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,16 @@ 2009-08-19 Eric Blake + popen-safer: prevent popen from clobbering std descriptors + * modules/popen-safer: New file. + * lib/popen-safer.c: Likewise. + * m4/stdio-safer.m4 (gl_POPEN_SAFER): New macro. + * lib/stdio--.h (popen): Provide override. + * lib/stdio-safer.h (popen_safer): Provide declaration. + * tests/test-popen.c (includes): Partially test this. + * modules/popen-safer-tests: New file, for more tests. + * tests/test-popen-safer.c: Likewise. + * MODULES.html.sh (file stream based Input/Output): Mention it. + tests: test some of the *-safer modules * modules/fopen-safer (Depends-on): Add fopen. * modules/fcntl-safer (Depends-on): Add fcntl. diff --git a/MODULES.html.sh b/MODULES.html.sh index cd23527c9..9d52e42bf 100755 --- a/MODULES.html.sh +++ b/MODULES.html.sh @@ -2538,6 +2538,7 @@ func_all_modules () func_module fwriting func_module getpass func_module getpass-gnu + func_module popen-safer func_module stdlib-safer func_module tmpfile-safer func_end_table diff --git a/lib/popen-safer.c b/lib/popen-safer.c new file mode 100644 index 000000000..2182a2c03 --- /dev/null +++ b/lib/popen-safer.c @@ -0,0 +1,85 @@ +/* Invoke popen, but avoid some glitches. + + Copyright (C) 2009 Free Software Foundation, Inc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +/* Written by Eric Blake. */ + +#include + +#include "stdio-safer.h" + +#include +#include +#include + +#include "cloexec.h" + +#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ +# define O_CLOEXEC O_NOINHERIT +#elif !defined O_CLOEXEC +# define O_CLOEXEC 0 +#endif + +/* Like open (name, flags | O_CLOEXEC), although not necessarily + atomic. FLAGS must not include O_CREAT. */ + +static int +open_noinherit (char const *name, int flags) +{ + int fd = open (name, flags | O_CLOEXEC); + if (0 <= fd && !O_CLOEXEC && set_cloexec_flag (fd, true) != 0) + { + int saved_errno = errno; + close (fd); + fd = -1; + errno = saved_errno; + } + return fd; +} + +/* Like popen, but do not return stdin, stdout, or stderr. */ + +FILE * +popen_safer (char const *cmd, char const *mode) +{ + /* Unfortunately, we cannot use the fopen_safer approach of using + fdopen (dup_safer (fileno (popen (cmd, mode)))), because stdio + libraries maintain hidden state tying the original fd to the pid + to wait on when using pclose (this hidden state is also used to + avoid fd leaks in subsequent popen calls). So, we instead + guarantee that all standard streams are open prior to the popen + call (even though this puts more pressure on open fds), so that + the original fd created by popen is safe. */ + FILE *fp; + int fd = open_noinherit ("/dev/null", O_RDONLY); + if (0 <= fd && fd <= STDERR_FILENO) + { + /* Maximum recursion depth is 3. */ + int saved_errno; + fp = popen_safer (cmd, mode); + saved_errno = errno; + close (fd); + errno = saved_errno; + } + else + { + /* Either all fd's are tied up, or fd is safe and the real popen + will reuse it. */ + close (fd); + fp = popen (cmd, mode); + } + return fp; +} diff --git a/lib/stdio--.h b/lib/stdio--.h index eafbdb127..ed90dda5a 100644 --- a/lib/stdio--.h +++ b/lib/stdio--.h @@ -29,3 +29,8 @@ # undef tmpfile # define tmpfile tmpfile_safer #endif + +#if GNULIB_POPEN_SAFER +# undef popen +# define popen popen_safer +#endif diff --git a/lib/stdio-safer.h b/lib/stdio-safer.h index c881d5a6a..dde22f104 100644 --- a/lib/stdio-safer.h +++ b/lib/stdio-safer.h @@ -1,6 +1,6 @@ /* Invoke stdio functions, but avoid some glitches. - Copyright (C) 2001, 2003, 2006 Free Software Foundation, Inc. + Copyright (C) 2001, 2003, 2006, 2009 Free Software Foundation, Inc. This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -20,4 +20,5 @@ #include FILE *fopen_safer (char const *, char const *); +FILE *popen_safer (char const *, char const *); FILE *tmpfile_safer (void); diff --git a/m4/stdio-safer.m4 b/m4/stdio-safer.m4 index 3d71452ea..9f9d7cc9a 100644 --- a/m4/stdio-safer.m4 +++ b/m4/stdio-safer.m4 @@ -1,5 +1,5 @@ -#serial 10 -dnl Copyright (C) 2002, 2005-2007 Free Software Foundation, Inc. +#serial 11 +dnl Copyright (C) 2002, 2005-2007, 2009 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, dnl with or without modifications, as long as this notice is preserved. @@ -9,6 +9,11 @@ AC_DEFUN([gl_FOPEN_SAFER], AC_LIBOBJ([fopen-safer]) ]) +AC_DEFUN([gl_POPEN_SAFER], +[ + AC_LIBOBJ([popen-safer]) +]) + AC_DEFUN([gl_TMPFILE_SAFER], [ AC_LIBOBJ([tmpfile-safer]) diff --git a/modules/popen-safer b/modules/popen-safer new file mode 100644 index 000000000..76bf4ee55 --- /dev/null +++ b/modules/popen-safer @@ -0,0 +1,29 @@ +Description: +popen function that avoids clobbering std{in,out,err}. + +Files: +lib/stdio--.h +lib/stdio-safer.h +lib/popen-safer.c +m4/stdio-safer.m4 + +Depends-on: +cloexec +open +popen +unistd-safer + +configure.ac: +gl_POPEN_SAFER +gl_MODULE_INDICATOR([popen-safer]) + +Makefile.am: + +Include: +"stdio-safer.h" + +License: +GPL + +Maintainer: +Eric Blake diff --git a/modules/popen-safer-tests b/modules/popen-safer-tests new file mode 100644 index 000000000..3dd67f24b --- /dev/null +++ b/modules/popen-safer-tests @@ -0,0 +1,12 @@ +Files: +tests/test-popen-safer.c + +Depends-on: +dup2 +sys_wait + +configure.ac: + +Makefile.am: +TESTS += test-popen-safer +check_PROGRAMS += test-popen-safer diff --git a/tests/test-popen-safer.c b/tests/test-popen-safer.c new file mode 100644 index 000000000..281dae9a7 --- /dev/null +++ b/tests/test-popen-safer.c @@ -0,0 +1,86 @@ +/* Test of opening a subcommand stream. + Copyright (C) 2009 Free Software Foundation, Inc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +/* Written by Eric Blake , 2009. */ + +#include + +/* Specification. */ +#include "stdio--.h" + +/* Helpers. */ +#include +#include +#include + +/* This test intentionally closes stderr. So, we arrange to have fd 10 + (outside the range of interesting fd's during the test) set up to + duplicate the original stderr. */ + +#define BACKUP_STDERR_FILENO 10 +static FILE *myerr; + +#define ASSERT(expr) \ + do \ + { \ + if (!(expr)) \ + { \ + fprintf (myerr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ + fflush (myerr); \ + abort (); \ + } \ + } \ + while (0) + +int +main (int argc, char **argv) +{ + FILE *fp; + int status; + + /* We close fd 2 later, so save it in fd 10. */ + if (dup2 (STDERR_FILENO, BACKUP_STDERR_FILENO) != BACKUP_STDERR_FILENO + || (myerr = fdopen (BACKUP_STDERR_FILENO, "w")) == NULL) + return 2; + + ASSERT (fp = popen ("exit 0", "r")); + ASSERT (STDERR_FILENO < fileno (fp)); + status = pclose (fp); + ASSERT (WIFEXITED (status)); + ASSERT (!WEXITSTATUS (status)); + + ASSERT (fclose (stdin) == 0); + ASSERT (fp = popen ("exit 0", "r")); + ASSERT (STDERR_FILENO < fileno (fp)); + status = pclose (fp); + ASSERT (WIFEXITED (status)); + ASSERT (!WEXITSTATUS (status)); + + ASSERT (fclose (stdout) == 0); + ASSERT (fp = popen ("exit 0", "r")); + ASSERT (STDERR_FILENO < fileno (fp)); + status = pclose (fp); + ASSERT (WIFEXITED (status)); + ASSERT (!WEXITSTATUS (status)); + + ASSERT (fclose (stderr) == 0); + ASSERT (fp = popen ("exit 0", "r")); + ASSERT (STDERR_FILENO < fileno (fp)); + status = pclose (fp); + ASSERT (WIFEXITED (status)); + ASSERT (!WEXITSTATUS (status)); + return 0; +} diff --git a/tests/test-popen.c b/tests/test-popen.c index 3d689e9a9..4e43bd70e 100644 --- a/tests/test-popen.c +++ b/tests/test-popen.c @@ -27,6 +27,10 @@ #include #include +#if GNULIB_POPEN_SAFER +# include "stdio--.h" +#endif + #define ASSERT(expr) \ do \ { \