a35d80
Fix SXID_ERASE behavior in setuid programs (BZ #27471)
a35d80
a35d80
When parse_tunables tries to erase a tunable marked as SXID_ERASE for
a35d80
setuid programs, it ends up setting the envvar string iterator
a35d80
incorrectly, because of which it may parse the next tunable
a35d80
incorrectly.  Given that currently the implementation allows malformed
a35d80
and unrecognized tunables pass through, it may even allow SXID_ERASE
a35d80
tunables to go through.
a35d80
a35d80
This change revamps the SXID_ERASE implementation so that:
a35d80
a35d80
- Only valid tunables are written back to the tunestr string, because
a35d80
  of which children of SXID programs will only inherit a clean list of
a35d80
  identified tunables that are not SXID_ERASE.
a35d80
a35d80
- Unrecognized tunables get scrubbed off from the environment and
a35d80
  subsequently from the child environment.
a35d80
a35d80
- This has the side-effect that a tunable that is not identified by
a35d80
  the setxid binary, will not be passed on to a non-setxid child even
a35d80
  if the child could have identified that tunable.  This may break
a35d80
  applications that expect this behaviour but expecting such tunables
a35d80
  to cross the SXID boundary is wrong.
a35d80
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
a35d80
a35d80
(cherry picked from commit 2ed18c5b534d9e92fc006202a5af0df6b72e7aca)
a35d80
a35d80
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
a35d80
index 4c9d36e3980758b9..bbc3679e3564a766 100644
a35d80
--- a/elf/dl-tunables.c
a35d80
+++ b/elf/dl-tunables.c
a35d80
@@ -178,6 +178,7 @@ parse_tunables (char *tunestr, char *valstring)
a35d80
     return;
a35d80
 
a35d80
   char *p = tunestr;
a35d80
+  size_t off = 0;
a35d80
 
a35d80
   while (true)
a35d80
     {
a35d80
@@ -191,7 +192,11 @@ parse_tunables (char *tunestr, char *valstring)
a35d80
       /* If we reach the end of the string before getting a valid name-value
a35d80
 	 pair, bail out.  */
a35d80
       if (p[len] == '\0')
a35d80
-	return;
a35d80
+	{
a35d80
+	  if (__libc_enable_secure)
a35d80
+	    tunestr[off] = '\0';
a35d80
+	  return;
a35d80
+	}
a35d80
 
a35d80
       /* We did not find a valid name-value pair before encountering the
a35d80
 	 colon.  */
a35d80
@@ -217,35 +222,28 @@ parse_tunables (char *tunestr, char *valstring)
a35d80
 
a35d80
 	  if (tunable_is_name (cur->name, name))
a35d80
 	    {
a35d80
-	      /* If we are in a secure context (AT_SECURE) then ignore the tunable
a35d80
-		 unless it is explicitly marked as secure.  Tunable values take
a35d80
-		 precendence over their envvar aliases.  */
a35d80
+	      /* If we are in a secure context (AT_SECURE) then ignore the
a35d80
+		 tunable unless it is explicitly marked as secure.  Tunable
a35d80
+		 values take precedence over their envvar aliases.  We write
a35d80
+		 the tunables that are not SXID_ERASE back to TUNESTR, thus
a35d80
+		 dropping all SXID_ERASE tunables and any invalid or
a35d80
+		 unrecognized tunables.  */
a35d80
 	      if (__libc_enable_secure)
a35d80
 		{
a35d80
-		  if (cur->security_level == TUNABLE_SECLEVEL_SXID_ERASE)
a35d80
+		  if (cur->security_level != TUNABLE_SECLEVEL_SXID_ERASE)
a35d80
 		    {
a35d80
-		      if (p[len] == '\0')
a35d80
-			{
a35d80
-			  /* Last tunable in the valstring.  Null-terminate and
a35d80
-			     return.  */
a35d80
-			  *name = '\0';
a35d80
-			  return;
a35d80
-			}
a35d80
-		      else
a35d80
-			{
a35d80
-			  /* Remove the current tunable from the string.  We do
a35d80
-			     this by overwriting the string starting from NAME
a35d80
-			     (which is where the current tunable begins) with
a35d80
-			     the remainder of the string.  We then have P point
a35d80
-			     to NAME so that we continue in the correct
a35d80
-			     position in the valstring.  */
a35d80
-			  char *q = &p[len + 1];
a35d80
-			  p = name;
a35d80
-			  while (*q != '\0')
a35d80
-			    *name++ = *q++;
a35d80
-			  name[0] = '\0';
a35d80
-			  len = 0;
a35d80
-			}
a35d80
+		      if (off > 0)
a35d80
+			tunestr[off++] = ':';
a35d80
+
a35d80
+		      const char *n = cur->name;
a35d80
+
a35d80
+		      while (*n != '\0')
a35d80
+			tunestr[off++] = *n++;
a35d80
+
a35d80
+		      tunestr[off++] = '=';
a35d80
+
a35d80
+		      for (size_t j = 0; j < len; j++)
a35d80
+			tunestr[off++] = value[j];
a35d80
 		    }
a35d80
 
a35d80
 		  if (cur->security_level != TUNABLE_SECLEVEL_NONE)
a35d80
@@ -258,9 +256,7 @@ parse_tunables (char *tunestr, char *valstring)
a35d80
 	    }
a35d80
 	}
a35d80
 
a35d80
-      if (p[len] == '\0')
a35d80
-	return;
a35d80
-      else
a35d80
+      if (p[len] != '\0')
a35d80
 	p += len + 1;
a35d80
     }
a35d80
 }
a35d80
diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c
a35d80
index a48281b175af6920..0b9b075c40598c6f 100644
a35d80
--- a/elf/tst-env-setuid-tunables.c
a35d80
+++ b/elf/tst-env-setuid-tunables.c
a35d80
@@ -45,11 +45,37 @@
a35d80
 const char *teststrings[] =
a35d80
 {
a35d80
   "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
a35d80
+  "glibc.malloc.check=2:glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
a35d80
+  "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096:glibc.malloc.check=2",
a35d80
+  "glibc.malloc.perturb=0x800",
a35d80
+  "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",
a35d80
+  "glibc.malloc.perturb=0x800:not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",
a35d80
+  "glibc.not_valid.check=2:glibc.malloc.mmap_threshold=4096",
a35d80
+  "not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",
a35d80
+  "glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096:glibc.malloc.check=2",
a35d80
+  "glibc.malloc.check=4:glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096",
a35d80
+  ":glibc.malloc.garbage=2:glibc.malloc.check=1",
a35d80
+  "glibc.malloc.check=1:glibc.malloc.check=2",
a35d80
+  "not_valid.malloc.check=2",
a35d80
+  "glibc.not_valid.check=2",
a35d80
 };
a35d80
 
a35d80
 const char *resultstrings[] =
a35d80
 {
a35d80
   "glibc.malloc.mmap_threshold=4096",
a35d80
+  "glibc.malloc.mmap_threshold=4096",
a35d80
+  "glibc.malloc.mmap_threshold=4096",
a35d80
+  "glibc.malloc.perturb=0x800",
a35d80
+  "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",
a35d80
+  "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",
a35d80
+  "glibc.malloc.mmap_threshold=4096",
a35d80
+  "glibc.malloc.mmap_threshold=4096",
a35d80
+  "",
a35d80
+  "",
a35d80
+  "",
a35d80
+  "",
a35d80
+  "",
a35d80
+  "",
a35d80
 };
a35d80
 
a35d80
 static int