commit 1bf38e7260c2cd3e40e118c1f1ea28de182dc751

Author: Siddhesh Poyarekar <siddhesh@sourceware.org>

Date:   Tue Mar 16 12:37:55 2021 +0530

    Fix SXID_ERASE behavior in setuid programs (BZ #27471)

    When parse_tunables tries to erase a tunable marked as SXID_ERASE for

    setuid programs, it ends up setting the envvar string iterator

    incorrectly, because of which it may parse the next tunable

    incorrectly.  Given that currently the implementation allows malformed

    and unrecognized tunables pass through, it may even allow SXID_ERASE

    tunables to go through.

    This change revamps the SXID_ERASE implementation so that:

    - Only valid tunables are written back to the tunestr string, because

      of which children of SXID programs will only inherit a clean list of

      identified tunables that are not SXID_ERASE.

    - Unrecognized tunables get scrubbed off from the environment and

      subsequently from the child environment.

    - This has the side-effect that a tunable that is not identified by

      the setxid binary, will not be passed on to a non-setxid child even

      if the child could have identified that tunable.  This may break

      applications that expect this behaviour but expecting such tunables

      to cross the SXID boundary is wrong.

    Reviewed-by: Carlos O'Donell <carlos@redhat.com>

    (cherry picked from commit 2ed18c5b534d9e92fc006202a5af0df6b72e7aca)

diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c

index 8b751dcf0deb0d01..8009e54ee5db32be 100644

--- a/elf/dl-tunables.c

+++ b/elf/dl-tunables.c

@@ -174,6 +174,7 @@ parse_tunables (char *tunestr, char *valstring)

     return;

   char *p = tunestr;

+  size_t off = 0;

   while (true)

     {

@@ -187,7 +188,11 @@ parse_tunables (char *tunestr, char *valstring)

       /* If we reach the end of the string before getting a valid name-value

 	 pair, bail out.  */

       if (p[len] == '\0')

-	return;

+	{

+	  if (__libc_enable_secure)

+	    tunestr[off] = '\0';

+	  return;

+	}

       /* We did not find a valid name-value pair before encountering the

 	 colon.  */

@@ -213,35 +218,28 @@ parse_tunables (char *tunestr, char *valstring)

 	  if (tunable_is_name (cur->name, name))

 	    {

-	      /* If we are in a secure context (AT_SECURE) then ignore the tunable

-		 unless it is explicitly marked as secure.  Tunable values take

-		 precedence over their envvar aliases.  */

+	      /* If we are in a secure context (AT_SECURE) then ignore the

+		 tunable unless it is explicitly marked as secure.  Tunable

+		 values take precedence over their envvar aliases.  We write

+		 the tunables that are not SXID_ERASE back to TUNESTR, thus

+		 dropping all SXID_ERASE tunables and any invalid or

+		 unrecognized tunables.  */

 	      if (__libc_enable_secure)

 		{

-		  if (cur->security_level == TUNABLE_SECLEVEL_SXID_ERASE)

+		  if (cur->security_level != TUNABLE_SECLEVEL_SXID_ERASE)

 		    {

-		      if (p[len] == '\0')

-			{

-			  /* Last tunable in the valstring.  Null-terminate and

-			     return.  */

-			  *name = '\0';

-			  return;

-			}

-		      else

-			{

-			  /* Remove the current tunable from the string.  We do

-			     this by overwriting the string starting from NAME

-			     (which is where the current tunable begins) with

-			     the remainder of the string.  We then have P point

-			     to NAME so that we continue in the correct

-			     position in the valstring.  */

-			  char *q = &p[len + 1];

-			  p = name;

-			  while (*q != '\0')

-			    *name++ = *q++;

-			  name[0] = '\0';

-			  len = 0;

-			}

+		      if (off > 0)

+			tunestr[off++] = ':';

+

+		      const char *n = cur->name;

+

+		      while (*n != '\0')

+			tunestr[off++] = *n++;

+

+		      tunestr[off++] = '=';

+

+		      for (size_t j = 0; j < len; j++)

+			tunestr[off++] = value[j];

 		    }

 		  if (cur->security_level != TUNABLE_SECLEVEL_NONE)

@@ -254,9 +252,7 @@ parse_tunables (char *tunestr, char *valstring)

 	    }

 	}

-      if (p[len] == '\0')

-	return;

-      else

+      if (p[len] != '\0')

 	p += len + 1;

     }

 }

diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c

index 3d523875b1e07a74..05619c9adc8b2698 100644

--- a/elf/tst-env-setuid-tunables.c

+++ b/elf/tst-env-setuid-tunables.c

@@ -45,11 +45,37 @@

 const char *teststrings[] =

 {

   "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",

+  "glibc.malloc.check=2:glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",

+  "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096:glibc.malloc.check=2",

+  "glibc.malloc.perturb=0x800",

+  "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",

+  "glibc.malloc.perturb=0x800:not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",

+  "glibc.not_valid.check=2:glibc.malloc.mmap_threshold=4096",

+  "not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",

+  "glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096:glibc.malloc.check=2",

+  "glibc.malloc.check=4:glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096",

+  ":glibc.malloc.garbage=2:glibc.malloc.check=1",

+  "glibc.malloc.check=1:glibc.malloc.check=2",

+  "not_valid.malloc.check=2",

+  "glibc.not_valid.check=2",

 };

 const char *resultstrings[] =

 {

   "glibc.malloc.mmap_threshold=4096",

+  "glibc.malloc.mmap_threshold=4096",

+  "glibc.malloc.mmap_threshold=4096",

+  "glibc.malloc.perturb=0x800",

+  "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",

+  "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",

+  "glibc.malloc.mmap_threshold=4096",

+  "glibc.malloc.mmap_threshold=4096",

+  "",

+  "",

+  "",

+  "",

+  "",

+  "",

 };

 static int