Hi Nikos,

Thanks for the suggestion.

But we need to distinguish native Windows and Cygwin.
Native Windows does not have a setuid concept, but Cygwin has. [1]

Also, in the code, the "#  ifdef _WIN32" conditional is wrong.
1) It is a compiler-dependent conditional. In order to treat the
   GCC and MSVC compilers the same way, you need
     #if (defined _WIN32 || defined __WIN32__)
2) On Cygwin, (defined _WIN32 || defined __WIN32__) evaluates to true
   depending of GCC flags. Which is not what we want, because the
   Cygwin setuid concept does not get eliminated by added some compiler
   flags.
The right conditional for checking for native Windows is therefore
#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__

Also, Paul, why not add an implementation for Solaris, HP-UX, AIX, etc.
as well?

Find attached a proposed implementation for all platforms. For your review.

Bruno

[1] 
http://pipeline.lbl.gov/code/3rd_party/licenses.win/cygwin-doc-1.4/cygwin-ug-net.html#ntsec-setuid
>From b117e55e41bb79b633cbd7992b9f7f532d3befcd Mon Sep 17 00:00:00 2001
From: Bruno Haible <[email protected]>
Date: Sun, 29 May 2016 12:54:32 +0200
Subject: [PATCH] secure_getenv: Port to many more platforms.

* m4/secure_getenv.m4 (gl_PREREQ_SECURE_GETENV): Also check for get*id
functions.
* lib/secure_getenv.c (secure_getenv): Add alternate implementations
for non-BSD Unix platforms and for native Windows.
* doc/glibc-functions/secure_getenv.texi: Remove known issue.
Prompted by a request from Nikos Mavrogiannopoulos.
---
 ChangeLog                              | 10 ++++++++++
 doc/glibc-functions/secure_getenv.texi |  4 ----
 lib/secure_getenv.c                    | 29 +++++++++++++++++++++--------
 m4/secure_getenv.m4                    |  1 +
 4 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 2d5a548..8189f4c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2016-05-29  Bruno Haible  <[email protected]>
+
+	secure_getenv: Port to many more platforms.
+	* m4/secure_getenv.m4 (gl_PREREQ_SECURE_GETENV): Also check for get*id
+	functions.
+	* lib/secure_getenv.c (secure_getenv): Add alternate implementations
+	for non-BSD Unix platforms and for native Windows.
+	* doc/glibc-functions/secure_getenv.texi: Remove known issue.
+	Prompted by a request from Nikos Mavrogiannopoulos.
+
 2016-05-27  Eric Blake  <[email protected]>
 
 	canonicalize: Fix broken probe for realpath.
diff --git a/doc/glibc-functions/secure_getenv.texi b/doc/glibc-functions/secure_getenv.texi
index 300285e..eeb3413 100644
--- a/doc/glibc-functions/secure_getenv.texi
+++ b/doc/glibc-functions/secure_getenv.texi
@@ -15,8 +15,4 @@ Interix 6.1, BeOS.
 
 Portability problems not fixed by Gnulib:
 @itemize
-@item
-On platforms other than glibc 2.0 and later, the Gnulib replacement
-function always returns a null pointer, even when invoked in a
-non-setuid program.
 @end itemize
diff --git a/lib/secure_getenv.c b/lib/secure_getenv.c
index b6021d5..821cb09 100644
--- a/lib/secure_getenv.c
+++ b/lib/secure_getenv.c
@@ -1,4 +1,4 @@
-/* Look up an environment variable more securely.
+/* Look up an environment variable, returning NULL in insecure situations.
 
    Copyright 2013-2016 Free Software Foundation, Inc.
 
@@ -20,22 +20,35 @@
 #include <stdlib.h>
 
 #if !HAVE___SECURE_GETENV
-# if HAVE_ISSETUGID
+# if HAVE_ISSETUGID || (HAVE_GETUID && HAVE_GETEUID && HAVE_GETGID && HAVE_GETEGID)
 #  include <unistd.h>
-# else
-#  undef issetugid
-#  define issetugid() 1
 # endif
 #endif
 
 char *
 secure_getenv (char const *name)
 {
-#if HAVE___SECURE_GETENV
+#if HAVE___SECURE_GETENV /* glibc */
   return __secure_getenv (name);
-#else
+#elif HAVE_ISSETUGID /* OS X, FreeBSD, NetBSD, OpenBSD */
   if (issetugid ())
-    return 0;
+    return NULL;
+  return getenv (name);
+#elif HAVE_GETUID && HAVE_GETEUID && HAVE_GETGID && HAVE_GETEGID /* other Unix */
+  if (geteuid () != getuid () || getegid () != getgid ())
+    return NULL;
   return getenv (name);
+#elif (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ /* native Windows */
+  /* On native Windows, there is no such concept as setuid or setgid binaries.
+     - Programs launched as system services have high privileges, but they don't
+       inherit environment variables from a user.
+     - Programs launched by a user with "Run as Administrator" have high
+       privileges and use the environment variables, but the user has been asked
+       whether he agrees.
+     - Programs launched by a user without "Run as Administrator" cannot gain
+       high privileges, therefore there is no risk. */
+  return getenv (name);
+#else
+  return NULL;
 #endif
 }
diff --git a/m4/secure_getenv.m4 b/m4/secure_getenv.m4
index 00194c8..3983173 100644
--- a/m4/secure_getenv.m4
+++ b/m4/secure_getenv.m4
@@ -22,4 +22,5 @@ AC_DEFUN([gl_PREREQ_SECURE_GETENV], [
   if test $ac_cv_func___secure_getenv = no; then
     AC_CHECK_FUNCS([issetugid])
   fi
+  AC_CHECK_FUNCS_ONCE([getuid geteuid getgid getegid])
 ])
-- 
2.6.4

Reply via email to