[PATCH] BZ #15754: CVE-2013-4788: PTR_MANGLE does not initialize to a random value for the pointer guard when compiling static executables

["Carlos O'Donell" <carlos at redhat dot com>]

CVE-2013-4788 glibc: PTR_MANGLE does not initialize to a 
random value for the pointer guard when compiling static 
executables.

It was reported in [1],[2] that glibc and eglibc suffer from
a flaw due to the PTR_MANGLE implementations.  As described 
by the reporter:
~~~
The vulnerability is caused due to the non initialization 
to a random value (it is always zero) of the "pointer guard" 
by the glibc only when generating static compiled executables. 
Dynamic executables are not affected. Pointer guard is used 
to mangle the content of sensible pointers (longjmp, signal 
handlers, etc.), if the pointer guard value is zero 
(non-initialized) then it is not effective. 
~~~
[1] http://hmarco.org/bugs/CVE-2013-4788.html
[2] http://www.openwall.com/lists/oss-security/2013/07/15/5

The following patch fixes the defect by initializing the
pointer guard for static applications.

An additional regression test, based on tst-stackguard1.c,
is added to check that the pointer guard is sufficiently
random and initialized for a static application.

Without the fix the test fails with:
~~~
tst-ptrguard1-static --command "tst-ptrguard1-static --child"
differences 0 defaults 0
pointer guard canaries are not randomized enough
nor equal to the default canary value
~~~

After the fix the test passes:
~~~
tst-ptrguard1-static --command "tst-ptrguard1-static --child"
differences 16 defaults 0
~~~

The non-static test passes before and after the patch
because the non-static case always has a random pointer guard.

This test only passes on x86-64, all other targets need to 
implement POINTER_CHK_GUARD in stackguard-macros.h to pass 
the test (and even build at this point). We might want to
rename stackguard-macros.h, but I didn't.

Given that we are frozen for 2.18 we could split this into
two pieces, one with the fix, another with the test case
once 2.19 reopens and machine maintainers can commit their
implementations of POINTER_CHK_GUARD.

Tested on x86-64 with no regressions.

Comments?

2013-07-19  Hector Marco  <hecmargi@upv.es>
            Ismael Ripoll  <iripoll@disca.upv.es>
	    Carlos O'Donell  <carlos@redhat.com>

        [BZ #15754]
        * csu/libc-start.c [!SHARED && !THREAD_SET_POINTER_GUARD]:
        Define __pointer_chk_guard_local.
        (LIBC_START_MAIN) [!SHARED]: Call _dl_setup_pointer_guard.
        Use THREAD_SET_POINTER_GUARD or set __pointer_chk_guard_local.

diff --git a/csu/libc-start.c b/csu/libc-start.c
index e5da3ef..c898d06 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -37,6 +37,12 @@ extern void __pthread_initialize_minimal (void);
    in thread local area.  */
 uintptr_t __stack_chk_guard attribute_relro;
 # endif
+# ifndef  THREAD_SET_POINTER_GUARD
+/* Only exported for architectures that don't store the pointer guard
+   value in thread local area.  */
+uintptr_t __pointer_chk_guard_local
+       attribute_relro attribute_hidden __attribute__ ((nocommon));
+# endif
 #endif
 
 #ifdef HAVE_PTR_NTHREADS
@@ -195,6 +201,16 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
 # else
   __stack_chk_guard = stack_chk_guard;
 # endif
+
+  /* Set up the pointer guard value.  */
+  uintptr_t pointer_chk_guard = _dl_setup_pointer_guard (_dl_random,
+                                                        stack_chk_guard);
+# ifdef THREAD_SET_POINTER_GUARD
+  THREAD_SET_POINTER_GUARD (pointer_chk_guard);
+# else
+  __pointer_chk_guard_local = pointer_chk_guard;
+# endif
+
 #endif
 
   /* Register the destructor of the dynamic linker if there is any.  */
---

2013-07-19  Carlos O'Donell  <carlos@redhat.com>

        [BZ #15754]
        * elf/Makefile (tests): Add tst-ptrguard1.
        (tests-static): Add tst-ptrguard1-static.
        (tst-ptrguard1-ARGS): Define.
        (tst-ptrguard1-static-ARGS): Define.
        * elf/tst-ptrguard1.c: New file.
        * elf/tst-ptrguard1-static.c: New file.
        * sysdeps/x86_64/stackguard-macros.h: Define POINTER_CHK_GUARD.

diff --git a/elf/Makefile b/elf/Makefile
index 3b58649..98834f4 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -121,7 +121,8 @@ endif
 tests = tst-tls1 tst-tls2 tst-tls9 tst-leaks1 \
        tst-array1 tst-array2 tst-array3 tst-array4 tst-array5
 tests-static = tst-tls1-static tst-tls2-static tst-stackguard1-static \
-              tst-leaks1-static tst-array1-static tst-array5-static
+              tst-leaks1-static tst-array1-static tst-array5-static \
+              tst-ptrguard1-static
 ifeq (yes,$(build-shared))
 tests-static += tst-tls9-static
 tst-tls9-static-ENV = \
@@ -145,7 +146,8 @@ tests += loadtest restest1 preloadtest loadfail multiload origtest resolvfail \
         tst-audit1 tst-audit2 tst-audit8 \
         tst-stackguard1 tst-addr1 tst-thrlock \
         tst-unique1 tst-unique2 tst-unique3 tst-unique4 \
-        tst-initorder tst-initorder2 tst-relsort1 tst-null-argv
+        tst-initorder tst-initorder2 tst-relsort1 tst-null-argv \
+        tst-ptrguard1
 #       reldep9
 test-srcs = tst-pathopt
 selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)
@@ -1016,6 +1018,9 @@ LDFLAGS-order2mod2.so = $(no-as-needed)
 tst-stackguard1-ARGS = --command "$(host-test-program-cmd) --child"
 tst-stackguard1-static-ARGS = --command "$(objpfx)tst-stackguard1-static --child"
 
+tst-ptrguard1-ARGS = --command "$(host-test-program-cmd) --child"
+tst-ptrguard1-static-ARGS = --command "$(objpfx)tst-ptrguard1-static --child"
+
 $(objpfx)tst-leaks1: $(libdl)
 $(objpfx)tst-leaks1-mem: $(objpfx)tst-leaks1.out
        $(common-objpfx)malloc/mtrace $(objpfx)tst-leaks1.mtrace > $@
diff --git a/elf/tst-ptrguard1-static.c b/elf/tst-ptrguard1-static.c
new file mode 100644
index 0000000..7aff3b7
--- /dev/null
+++ b/elf/tst-ptrguard1-static.c
@@ -0,0 +1 @@
+#include "tst-ptrguard1.c"
diff --git a/elf/tst-ptrguard1.c b/elf/tst-ptrguard1.c
new file mode 100644
index 0000000..899e2a9
--- /dev/null
+++ b/elf/tst-ptrguard1.c
@@ -0,0 +1,201 @@
+/* Copyright (C) 2013 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/wait.h>
+#include <stackguard-macros.h>
+#include <tls.h>
+#include <unistd.h>
+
+#ifndef POINTER_CHK_GUARD
+# error Machine must define POINTER_CHK_GUARD.
+#endif
+
+static const char *command;
+static bool child;
+static uintptr_t ptr_chk_guard_copy;
+static bool ptr_chk_guard_copy_set;
+static int fds[2];
+
+static void __attribute__ ((constructor))
+con (void)
+{
+  ptr_chk_guard_copy = POINTER_CHK_GUARD;
+  ptr_chk_guard_copy_set = true;
+}
+
+static int
+uintptr_t_cmp (const void *a, const void *b)
+{
+  if (*(uintptr_t *) a < *(uintptr_t *) b)
+    return 1;
+  if (*(uintptr_t *) a > *(uintptr_t *) b)
+    return -1;
+  return 0;
+}
+
+static int
+do_test (void)
+{
+  if (!ptr_chk_guard_copy_set)
+    {
+      puts ("constructor has not been run");
+      return 1;
+    }
+
+  if (ptr_chk_guard_copy != POINTER_CHK_GUARD)
+    {
+      puts ("POINTER_CHK_GUARD changed between constructor and do_test");
+      return 1;
+    }
+
+  if (child)
+    {
+      write (2, &ptr_chk_guard_copy, sizeof (ptr_chk_guard_copy));
+      return 0;
+    }
+
+  if (command == NULL)
+    {
+      puts ("missing --command or --child argument");
+      return 1;
+    }
+
+#define N 16
+  uintptr_t child_ptr_chk_guards[N + 1];
+  child_ptr_chk_guards[N] = ptr_chk_guard_copy;
+  int i;
+  for (i = 0; i < N; ++i)
+    {
+      if (pipe (fds) < 0)
+       {
+         printf ("couldn't create pipe: %m\n");
+         return 1;
+       }
+
+      pid_t pid = fork ();
+      if (pid < 0)
+       {
+         printf ("fork failed: %m\n");
+         return 1;
+       }
+
+      if (!pid)
+       {
+         if (ptr_chk_guard_copy != POINTER_CHK_GUARD)
+           {
+             puts ("POINTER_CHK_GUARD changed after fork");
+             exit (1);
+           }
+
+         close (fds[0]);
+         close (2);
+         dup2 (fds[1], 2);
+         close (fds[1]);
+
+         system (command);
+         exit (0);
+       }
+
+      close (fds[1]);
+
+      if (TEMP_FAILURE_RETRY (read (fds[0], &child_ptr_chk_guards[i],
+                                   sizeof (uintptr_t))) != sizeof (uintptr_t))
+       {
+         puts ("could not read ptr_chk_guard value from child");
+         return 1;
+       }
+
+      close (fds[0]);
+
+      pid_t termpid;
+      int status;
+      termpid = TEMP_FAILURE_RETRY (waitpid (pid, &status, 0));
+      if (termpid == -1)
+       {
+         printf ("waitpid failed: %m\n");
+         return 1;
+       }
+      else if (termpid != pid)
+       {
+         printf ("waitpid returned %ld != %ld\n",
+                 (long int) termpid, (long int) pid);
+         return 1;
+       }
+      else if (!WIFEXITED (status) || WEXITSTATUS (status))
+       {
+         puts ("child hasn't exited with exit status 0");
+         return 1;
+       }
+    }
+
+  qsort (child_ptr_chk_guards, N + 1, sizeof (uintptr_t), uintptr_t_cmp);
+
+  /* The default pointer guard is the same as the default stack guard.
+     They are only set to default if dl_random is NULL.  */
+  uintptr_t default_guard = 0;
+  unsigned char *p = (unsigned char *) &default_guard;
+  p[sizeof (uintptr_t) - 1] = 255;
+  p[sizeof (uintptr_t) - 2] = '\n';
+  p[0] = 0;
+
+  /* Test if the pointer guard canaries are either randomized,
+     or equal to the default pointer guard canary value.
+     Even with randomized pointer guards it might happen
+     that the random number generator generates the same
+     values, but if that happens in more than half from
+     the 16 runs, something is very wrong.  */
+  int ndifferences = 0;
+  int ndefaults = 0;
+  for (i = 0; i < N; ++i)
+    {
+      if (child_ptr_chk_guards[i] != child_ptr_chk_guards[i+1])
+       ndifferences++;
+      else if (child_ptr_chk_guards[i] == default_guard)
+       ndefaults++;
+    }
+
+  printf ("differences %d defaults %d\n", ndifferences, ndefaults);
+
+  if (ndifferences < N / 2 && ndefaults < N / 2)
+    {
+      puts ("pointer guard canaries are not randomized enough");
+      puts ("nor equal to the default canary value");
+      return 1;
+    }
+
+  return 0;
+}
+
+#define OPT_COMMAND    10000
+#define OPT_CHILD      10001
+#define CMDLINE_OPTIONS        \
+  { "command", required_argument, NULL, OPT_COMMAND },  \
+  { "child", no_argument, NULL, OPT_CHILD },
+#define CMDLINE_PROCESS        \
+  case OPT_COMMAND:    \
+    command = optarg;  \
+    break;             \
+  case OPT_CHILD:      \
+    child = true;      \
+    break;
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/sysdeps/x86_64/stackguard-macros.h b/sysdeps/x86_64/stackguard-macros.h
index d7fedb3..1948800 100644
--- a/sysdeps/x86_64/stackguard-macros.h
+++ b/sysdeps/x86_64/stackguard-macros.h
@@ -4,3 +4,8 @@
   ({ uintptr_t x;                                              \
      asm ("mov %%fs:%c1, %0" : "=r" (x)                                \
          : "i" (offsetof (tcbhead_t, stack_guard))); x; })
+
+#define POINTER_CHK_GUARD \
+  ({ uintptr_t x;                                              \
+     asm ("mov %%fs:%c1, %0" : "=r" (x)                                \
+         : "i" (offsetof (tcbhead_t, pointer_guard))); x; })
---

Cheers,
Carlos.

Cheers,
Carlos.