[PATCH 2/2] Erase GLIBC_TUNABLES for setxid processes when tunables is disabled

[Siddhesh Poyarekar <siddhesh at sourceware dot org>]

A setxid program that uses a glibc with tunables disabled may pass on
GLIBC_TUNABLES as is to its child processes.  If the child process
ends up using a different glibc that has tunables enabled, it will end
up getting access to unsafe tunables.  To fix this, remove
GLIBC_TUNABLES from the environment for setxid process.

In addition to this, I'll post a patch for earlier releases (2.24 and
older) to add GLIBC_TUNABLES to unsecure_envvars (is unsecure even a
word?) so that they too don't end up passing on unsafe tunables.
---
 elf/dl-tunables.c | 40 +++++-----------------------------------
 elf/dl-tunables.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 52 insertions(+), 37 deletions(-)

diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index c3d4c24..02aa864 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -31,25 +31,9 @@
 #include "dl-tunables.h"
 
 #if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring
-# define GLIBC_TUNABLES "GLIBC_TUNABLES"
+# define GLIBC_TUNABLES TUNABLES_VALSTRING_ENVVAR
 #endif
 
-/* Compare environment or tunable names, bounded by the name hardcoded in
-   glibc.  */
-static bool
-is_name (const char *orig, const char *envname)
-{
-  for (;*orig != '\0' && *envname != '\0'; envname++, orig++)
-    if (*orig != *envname)
-      break;
-
-  /* The ENVNAME is immediately followed by a value.  */
-  if (*orig == '\0' && *envname == '=')
-    return true;
-  else
-    return false;
-}
-
 static char **
 get_next_env (char **envp, char **name, size_t *namelen, char **val,
 	      char ***prev_envp)
@@ -405,24 +389,10 @@ __tunables_init (char **envp)
 		{
 		  if (cur->security_level == TUNABLE_SECLEVEL_SXID_ERASE)
 		    {
-		      /* Erase the environment variable.  */
-		      char **ep = prev_envp;
-
-		      while (*ep != NULL)
-			{
-			  if (is_name (name, *ep))
-			    {
-			      char **dp = ep;
-
-			      do
-				dp[0] = dp[1];
-			      while (*dp++);
-			    }
-			  else
-			    ++ep;
-			}
-		      /* Reset the iterator so that we read the environment again
-			 from the point we erased.  */
+		      /* Erase the environment variable and then reset the
+			 iterator so that we read the environment again from
+			 the point we erased.  */
+		      tunables_delete_env (name, prev_envp)
 		      envp = prev_envp;
 		    }
 
diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
index f33adfb..589e22f 100644
--- a/elf/dl-tunables.h
+++ b/elf/dl-tunables.h
@@ -21,12 +21,57 @@
 #ifndef _TUNABLES_H_
 #define _TUNABLES_H_
 
+#define TUNABLES_VALSTRING_ENVVAR "GLIBC_TUNABLES"
+
+/* Compare environment or tunable names, bounded by the name hardcoded in
+   glibc.  */
+static inline bool
+__always_inline
+is_name (const char *orig, const char *envname)
+{
+  for (;*orig != '\0' && *envname != '\0'; envname++, orig++)
+    if (*orig != *envname)
+      break;
+
+  /* The ENVNAME is immediately followed by a value.  */
+  if (*orig == '\0' && *envname == '=')
+    return true;
+  else
+    return false;
+}
+
+static inline void
+__always_inline
+tunables_delete_env (const char *name, char **envp)
+{
+  if (envp == NULL)
+    return;
+
+  while (*envp != NULL)
+    {
+      if (is_name (name, *envp))
+	{
+	  char **dp = envp;
+
+	  do
+	    dp[0] = dp[1];
+	  while (*dp++);
+	}
+      else
+	++envp;
+    }
+}
+
 #if !HAVE_TUNABLES
 static inline void
 __always_inline
-__tunables_init (char **unused __attribute__ ((unused)))
+__tunables_init (char **envp)
 {
-  /* This is optimized out if tunables are not enabled.  */
+  /* If tunables is not enabled, we want to make sure that we don't pass on
+     insecure tunables to child processes of setxid processes, so just drop
+     GLIBC_TUNABLES from the environment.  */
+  if (__libc_enable_secure)
+    tunables_delete_env (TUNABLES_VALSTRING_ENVVAR, envp);
 }
 #else
 
-- 
2.7.4