From 8dffeb6f35aff7e201bed55c43ea83b62139803e Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Sat, 14 Nov 2009 22:13:10 -0700 Subject: [PATCH 1/1] setenv, unsetenv: work around various bugs POSIX requires setenv(NULL,"",0), setenv("a=b","",0), unsetenv(NULL), and unsetenv("a=b") to fail with EINVAL, but many BSD implementations lack validation. The gnulib replacement for void unsetenv did not do validation, and the replacement for setenv was out of sync with glibc. Also, some BSD implementations of setenv("a","==",1) eat the leading '='. See also some recent Austin Group interpretations on environ: http://austingroupbugs.net/view.php?id=167 http://austingroupbugs.net/view.php?id=185 * lib/setenv.c (setenv) [!HAVE_SETENV]: Resync from glibc. (setenv) [HAVE_SETENV]: Work around bugs. * lib/unsetenv.c (unsetenv) [HAVE_UNSETENV]: Work around bugs. * m4/setenv.m4 (gl_FUNC_SETENV_SEPARATE, gl_FUNC_UNSETENV): Check for bugs. (gl_FUNC_SETENV): Write in terms of gl_FUNC_SETENV_SEPARATE. * m4/environ.m4 (gl_ENVIRON): Avoid expand-before-require. * m4/stdlib_h.m4 (gl_STDLIB_H_DEFAULTS): Update defaults. * modules/stdlib (Makefile.am): Update substitutions. * lib/stdlib.in.h (setenv, unsetenv): Update prototypes. * doc/posix-functions/setenv.texi (setenv): Document the bugs. * doc/posix-functions/unsetenv.texi (unsetenv): Likewise. * modules/setenv-tests: New test. * modules/unsetenv-tests: Likewise. * tests/test-setenv.c: New file. * tests/test-unsetenv.c: Likewise. Signed-off-by: Eric Blake --- ChangeLog | 20 ++++++++++++ doc/posix-functions/setenv.texi | 11 +++++-- doc/posix-functions/unsetenv.texi | 4 +++ lib/setenv.c | 54 ++++++++++++++++++++++++++++++-- lib/stdlib.in.h | 30 +++++++++++++----- lib/unsetenv.c | 27 +++++++++++++++- m4/environ.m4 | 4 +-- m4/setenv.m4 | 32 ++++++++++++++----- m4/stdlib_h.m4 | 3 +- modules/setenv-tests | 10 ++++++ modules/stdlib | 3 +- modules/unsetenv-tests | 11 +++++++ tests/test-setenv.c | 60 ++++++++++++++++++++++++++++++++++++ tests/test-unsetenv.c | 65 +++++++++++++++++++++++++++++++++++++++ 14 files changed, 308 insertions(+), 26 deletions(-) create mode 100644 modules/setenv-tests create mode 100644 modules/unsetenv-tests create mode 100644 tests/test-setenv.c create mode 100644 tests/test-unsetenv.c diff --git a/ChangeLog b/ChangeLog index 35daa343f..0c56777b4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,23 @@ +2009-11-16 Eric Blake + + setenv, unsetenv: work around various bugs + * lib/setenv.c (setenv) [!HAVE_SETENV]: Resync from glibc. + (setenv) [HAVE_SETENV]: Work around bugs. + * lib/unsetenv.c (unsetenv) [HAVE_UNSETENV]: Work around bugs. + * m4/setenv.m4 (gl_FUNC_SETENV_SEPARATE, gl_FUNC_UNSETENV): Check + for bugs. + (gl_FUNC_SETENV): Write in terms of gl_FUNC_SETENV_SEPARATE. + * m4/environ.m4 (gl_ENVIRON): Avoid expand-before-require. + * m4/stdlib_h.m4 (gl_STDLIB_H_DEFAULTS): Update defaults. + * modules/stdlib (Makefile.am): Update substitutions. + * lib/stdlib.in.h (setenv, unsetenv): Update prototypes. + * doc/posix-functions/setenv.texi (setenv): Document the bugs. + * doc/posix-functions/unsetenv.texi (unsetenv): Likewise. + * modules/setenv-tests: New test. + * modules/unsetenv-tests: Likewise. + * tests/test-setenv.c: New file. + * tests/test-unsetenv.c: Likewise. + 2009-11-16 Jim Meyering version-etc: relax license to LGPLv3+ diff --git a/doc/posix-functions/setenv.texi b/doc/posix-functions/setenv.texi index 7a90cdbbf..87c9e2d7c 100644 --- a/doc/posix-functions/setenv.texi +++ b/doc/posix-functions/setenv.texi @@ -11,11 +11,16 @@ Portability problems fixed by Gnulib: @item This function is missing on some platforms: AIX 4.3.2, HP-UX 11, IRIX 6.5, Solaris 9, mingw, BeOS. +@item +On some platforms, this function does not fail with @samp{EINVAL} when +passed a null pointer, an empty string, or a string containing @samp{=}: +FreeBSD 6.0, NetBSD 1.6, OpenBSD 3.8, Cygwin 1.5.x. +@item +On some platforms, this function removes a leading @samp{=} from the +value argument: +Cygwin 1.5.x. @end itemize Portability problems not fixed by Gnulib: @itemize -@item -In some versions of glibc (e.g.@: 2.3.3), @code{setenv} doesn't fail if the -first argument contains a @samp{=} character. @end itemize diff --git a/doc/posix-functions/unsetenv.texi b/doc/posix-functions/unsetenv.texi index 04c4fe365..f3566278d 100644 --- a/doc/posix-functions/unsetenv.texi +++ b/doc/posix-functions/unsetenv.texi @@ -15,6 +15,10 @@ AIX 5.1, HP-UX 11, IRIX 6.5, Solaris 9, mingw, BeOS. This function has the return type @samp{void} instead of @samp{int} on some platforms: MacOS X 10.3, FreeBSD 6.0, NetBSD 1.6, OpenBSD 3.8, OSF/1 5.1. +@item +On some platforms, this function does not fail with @samp{EINVAL} when +passed a null pointer, an empty string, or a string containing @samp{=}: +FreeBSD 6.0, NetBSD 1.6, OpenBSD 3.8. @end itemize Portability problems not fixed by Gnulib: diff --git a/lib/setenv.c b/lib/setenv.c index 83b52b88b..0df5b21b9 100644 --- a/lib/setenv.c +++ b/lib/setenv.c @@ -1,4 +1,4 @@ -/* Copyright (C) 1992,1995-1999,2000-2003,2005-2008 Free Software Foundation, Inc. +/* Copyright (C) 1992,1995-1999,2000-2003,2005-2009 Free Software Foundation, Inc. This file is part of the GNU C Library. This program is free software: you can redistribute it and/or modify @@ -32,12 +32,12 @@ # include #endif -#if _LIBC || !HAVE_SETENV - #if !_LIBC # include "malloca.h" #endif +#if _LIBC || !HAVE_SETENV + #if !_LIBC # define __environ environ #endif @@ -281,6 +281,12 @@ __add_to_environ (const char *name, const char *value, const char *combined, int setenv (const char *name, const char *value, int replace) { + if (name == NULL || *name == '\0' || strchr (name, '=') != NULL) + { + __set_errno (EINVAL); + return -1; + } + return __add_to_environ (name, value, NULL, replace); } @@ -328,3 +334,45 @@ weak_alias (__clearenv, clearenv) #endif #endif /* _LIBC || !HAVE_SETENV */ + +/* The rest of this file is called into use when replacing an existing + but buggy setenv. Known bugs include failure to diagnose invalid + name, and consuming a leading '=' from value. */ +#if HAVE_SETENV + +# undef setenv +# define STREQ(a, b) (strcmp (a, b) == 0) + +int +rpl_setenv (const char *name, const char *value, int replace) +{ + int result; + if (!name || !*name || strchr (name, '=')) + { + errno = EINVAL; + return -1; + } + /* Call the real setenv even if replace is 0, in case implementation + has underlying data to update, such as when environ changes. */ + result = setenv (name, value, replace); + if (result == 0 && replace && *value == '=') + { + char *tmp = getenv (name); + if (!STREQ (tmp, value)) + { + int saved_errno; + size_t len = strlen (value); + tmp = malloca (len + 2); + /* Since leading '=' is eaten, double it up. */ + *tmp = '='; + memcpy (tmp + 1, value, len + 1); + result = setenv (name, tmp, replace); + saved_errno = errno; + freea (tmp); + errno = saved_errno; + } + } + return result; +} + +#endif /* HAVE_SETENV */ diff --git a/lib/stdlib.in.h b/lib/stdlib.in.h index e2c6bbf35..dd15ac05b 100644 --- a/lib/stdlib.in.h +++ b/lib/stdlib.in.h @@ -384,11 +384,21 @@ extern int rpmatch (const char *response); #endif #if @GNULIB_SETENV@ -# if !@HAVE_SETENV@ +# if @REPLACE_SETENV@ +# undef setenv +# define setenv rpl_setenv +# endif +# if !@HAVE_SETENV@ || @REPLACE_SETENV@ /* Set NAME to VALUE in the environment. If REPLACE is nonzero, overwrite an existing value. */ extern int setenv (const char *name, const char *value, int replace); # endif +#elif defined GNULIB_POSIXCHECK +# undef setenv +# define setenv(n,v,o) \ + (GL_LINK_WARNING ("setenv is unportable - " \ + "use gnulib module setenv for portability"), \ + setenv (n, v, o)) #endif #if @GNULIB_STRTOD@ @@ -448,16 +458,20 @@ extern unsigned long long strtoull (const char *string, char **endptr, int base) #endif #if @GNULIB_UNSETENV@ -# if @HAVE_UNSETENV@ -# if @VOID_UNSETENV@ -/* On some systems, unsetenv() returns void. - This is the case for MacOS X 10.3, FreeBSD 4.8, NetBSD 1.6, OpenBSD 3.4. */ -# define unsetenv(name) ((unsetenv)(name), 0) -# endif -# else +# if @REPLACE_UNSETENV@ +# undef unsetenv +# define unsetenv rpl_unsetenv +# endif +# if !@HAVE_UNSETENV@ || @REPLACE_UNSETENV@ /* Remove the variable NAME from the environment. */ extern int unsetenv (const char *name); # endif +#elif defined GNULIB_POSIXCHECK +# undef unsetenv +# define unsetenv(n) \ + (GL_LINK_WARNING ("unsetenv is unportable - " \ + "use gnulib module unsetenv for portability"), \ + unsetenv (n)) #endif #ifdef __cplusplus diff --git a/lib/unsetenv.c b/lib/unsetenv.c index 73ea878a6..75670119d 100644 --- a/lib/unsetenv.c +++ b/lib/unsetenv.c @@ -1,4 +1,4 @@ -/* Copyright (C) 1992,1995-1999,2000-2002,2005-2008 Free Software Foundation, Inc. +/* Copyright (C) 1992,1995-1999,2000-2002,2005-2009 Free Software Foundation, Inc. This file is part of the GNU C Library. This program is free software: you can redistribute it and/or modify @@ -47,6 +47,7 @@ __libc_lock_define_initialized (static, envlock) # define unsetenv __unsetenv #endif +#if _LIBC || !HAVE_UNSETENV int unsetenv (const char *name) @@ -88,3 +89,27 @@ unsetenv (const char *name) # undef unsetenv weak_alias (__unsetenv, unsetenv) #endif + +#else /* HAVE_UNSETENV */ + +# undef unsetenv + +/* Call the underlying unsetenv, in case there is hidden bookkeeping + that needs updating beyond just modifying environ. */ +int +rpl_unsetenv (const char *name) +{ + int result = 0; + if (!name || !*name || strchr (name, '=')) + { + errno = EINVAL; + return -1; + } +# if !VOID_UNSETENV + result = +# endif + unsetenv (name); + return result; +} + +#endif /* HAVE_UNSETENV */ diff --git a/m4/environ.m4 b/m4/environ.m4 index b17bb60a7..180382020 100644 --- a/m4/environ.m4 +++ b/m4/environ.m4 @@ -1,10 +1,10 @@ -# environ.m4 serial 2 +# environ.m4 serial 3 dnl Copyright (C) 2001-2004, 2006-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. -AC_DEFUN([gl_ENVIRON], +AC_DEFUN_ONCE([gl_ENVIRON], [ AC_REQUIRE([gl_UNISTD_H_DEFAULTS]) dnl Persuade glibc to declare environ. diff --git a/m4/setenv.m4 b/m4/setenv.m4 index e28407ee1..ca146d2c0 100644 --- a/m4/setenv.m4 +++ b/m4/setenv.m4 @@ -1,4 +1,4 @@ -# setenv.m4 serial 11 +# setenv.m4 serial 12 dnl Copyright (C) 2001-2004, 2006-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, @@ -6,12 +6,9 @@ dnl with or without modifications, as long as this notice is preserved. AC_DEFUN([gl_FUNC_SETENV], [ - AC_REQUIRE([gl_STDLIB_H_DEFAULTS]) - AC_CHECK_FUNCS_ONCE([setenv]) - if test $ac_cv_func_setenv = no; then - HAVE_SETENV=0 + AC_REQUIRE([gl_FUNC_SETENV_SEPARATE]) + if test $HAVE_SETENV$REPLACE_SETENV != 10; then AC_LIBOBJ([setenv]) - gl_PREREQ_SETENV fi ]) @@ -22,6 +19,24 @@ AC_DEFUN([gl_FUNC_SETENV_SEPARATE], AC_CHECK_FUNCS_ONCE([setenv]) if test $ac_cv_func_setenv = no; then HAVE_SETENV=0 + else + AC_CACHE_CHECK([whether setenv validates arguments], + [gl_cv_func_setenv_works], + [AC_RUN_IFELSE([AC_LANG_PROGRAM([[ + #include + #include + ]], [[ + if (setenv (NULL, "", 0) != -1) return 1; + if (errno != EINVAL) return 2; + if (setenv ("a", "=", 1) != 0) return 3; + if (strcmp (getenv ("a"), "=") != 0) return 4; + ]])], + [gl_cv_func_setenv_works=yes], [gl_cv_func_setenv_works=no], + [gl_cv_func_setenv_works="guessing no"])]) + if test "$gl_cv_func_setenv_works" != yes; then + REPLACE_SETENV=1 + AC_LIBOBJ([setenv]) + fi fi gl_PREREQ_SETENV ]) @@ -48,7 +63,10 @@ int unsetenv(); #endif ], , gt_cv_func_unsetenv_ret='int', gt_cv_func_unsetenv_ret='void')]) if test $gt_cv_func_unsetenv_ret = 'void'; then - VOID_UNSETENV=1 + AC_DEFINE([VOID_UNSETENV], [1], [Define to 1 if unsetenv returns void + instead of int.]) + REPLACE_UNSETENV=1 + AC_LIBOBJ([unsetenv]) fi fi ]) diff --git a/m4/stdlib_h.m4 b/m4/stdlib_h.m4 index 4556ac04c..10e010e42 100644 --- a/m4/stdlib_h.m4 +++ b/m4/stdlib_h.m4 @@ -80,6 +80,7 @@ AC_DEFUN([gl_STDLIB_H_DEFAULTS], REPLACE_MKSTEMP=0; AC_SUBST([REPLACE_MKSTEMP]) REPLACE_PUTENV=0; AC_SUBST([REPLACE_PUTENV]) REPLACE_REALPATH=0; AC_SUBST([REPLACE_REALPATH]) + REPLACE_SETENV=0; AC_SUBST([REPLACE_SETENV]) REPLACE_STRTOD=0; AC_SUBST([REPLACE_STRTOD]) - VOID_UNSETENV=0; AC_SUBST([VOID_UNSETENV]) + REPLACE_UNSETENV=0; AC_SUBST([REPLACE_UNSETENV]) ]) diff --git a/modules/setenv-tests b/modules/setenv-tests new file mode 100644 index 000000000..9c923ac92 --- /dev/null +++ b/modules/setenv-tests @@ -0,0 +1,10 @@ +Files: +tests/test-setenv.c + +Depends-on: + +configure.ac: + +Makefile.am: +TESTS += test-setenv +check_PROGRAMS += test-setenv diff --git a/modules/stdlib b/modules/stdlib index 2e0408806..ddf2b7ac3 100644 --- a/modules/stdlib +++ b/modules/stdlib @@ -73,8 +73,9 @@ stdlib.h: stdlib.in.h -e 's|@''REPLACE_MKSTEMP''@|$(REPLACE_MKSTEMP)|g' \ -e 's|@''REPLACE_PUTENV''@|$(REPLACE_PUTENV)|g' \ -e 's|@''REPLACE_REALPATH''@|$(REPLACE_REALPATH)|g' \ + -e 's|@''REPLACE_SETENV''@|$(REPLACE_SETENV)|g' \ -e 's|@''REPLACE_STRTOD''@|$(REPLACE_STRTOD)|g' \ - -e 's|@''VOID_UNSETENV''@|$(VOID_UNSETENV)|g' \ + -e 's|@''REPLACE_UNSETENV''@|$(REPLACE_UNSETENV)|g' \ -e '/definition of GL_LINK_WARNING/r $(LINK_WARNING_H)' \ < $(srcdir)/stdlib.in.h; \ } > $@-t && \ diff --git a/modules/unsetenv-tests b/modules/unsetenv-tests new file mode 100644 index 000000000..94fc504ae --- /dev/null +++ b/modules/unsetenv-tests @@ -0,0 +1,11 @@ +Files: +tests/test-unsetenv.c + +Depends-on: +putenv + +configure.ac: + +Makefile.am: +TESTS += test-unsetenv +check_PROGRAMS += test-unsetenv diff --git a/tests/test-setenv.c b/tests/test-setenv.c new file mode 100644 index 000000000..61be8381b --- /dev/null +++ b/tests/test-setenv.c @@ -0,0 +1,60 @@ +/* Tests of setenv. + 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 + +#include + +#include +#include +#include +#include + +#define ASSERT(expr) \ + do \ + { \ + if (!(expr)) \ + { \ + fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ + fflush (stderr); \ + abort (); \ + } \ + } \ + while (0) + +int +main (void) +{ + /* Test overwriting. */ + ASSERT (setenv ("a", "==", -1) == 0); + ASSERT (setenv ("a", "2", 0) == 0); + ASSERT (strcmp (getenv ("a"), "==") == 0); + + /* Required to fail with EINVAL. */ + errno = 0; + ASSERT (setenv ("", "", 1) == -1); + ASSERT (errno == EINVAL); + errno = 0; + ASSERT (setenv ("a=b", "", 0) == -1); + ASSERT (errno == EINVAL); + errno = 0; + ASSERT (setenv (NULL, "", 0) == -1); + ASSERT (errno == EINVAL); + + return 0; +} diff --git a/tests/test-unsetenv.c b/tests/test-unsetenv.c new file mode 100644 index 000000000..11af82c99 --- /dev/null +++ b/tests/test-unsetenv.c @@ -0,0 +1,65 @@ +/* Tests of unsetenv. + 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 + +#include + +#include +#include +#include +#include + +#define ASSERT(expr) \ + do \ + { \ + if (!(expr)) \ + { \ + fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ + fflush (stderr); \ + abort (); \ + } \ + } \ + while (0) + +int +main (void) +{ + char entry[] = "b=2"; + + /* Test removal when multiple entries present. */ + ASSERT (putenv ("a=1") == 0); + ASSERT (putenv (entry) == 0); + entry[0] = 'a'; /* Unspecified what getenv("a") would be at this point. */ + ASSERT (unsetenv ("a") == 0); /* Both entries will be removed. */ + ASSERT (getenv ("a") == NULL); + ASSERT (unsetenv ("a") == 0); + + /* Required to fail with EINVAL. */ + errno = 0; + ASSERT (unsetenv ("") == -1); + ASSERT (errno == EINVAL); + errno = 0; + ASSERT (unsetenv ("a=b") == -1); + ASSERT (errno == EINVAL); + errno = 0; + ASSERT (unsetenv (NULL) == -1); + ASSERT (errno == EINVAL); + + return 0; +} -- 2.11.0