BZ #15754: CVE-2013-4788

The pointer guard used for pointer mangling was not initialized for
static applications resulting in the security feature being disabled.
The pointer guard is now correctly initialized to a random value for
static applications. Existing static applications need to be
recompiled to take advantage of the fix.

The test tst-ptrguard1-static and tst-ptrguard1 add regression
coverage to ensure the pointer guards are sufficiently random
and initialized to a default value.

19 files changed, 351 insertions, 5 deletions

diff --git a/ChangeLog b/ChangeLog
index 4282494032..5972ae01f9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,33 @@
+2013-09-23  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.
+	* sysdeps/i386/stackguard-macros.h: Likewise.
+	* sysdeps/powerpc/powerpc32/stackguard-macros.h: Likewise.
+	* sysdeps/powerpc/powerpc64/stackguard-macros.h: Likewise.
+	* sysdeps/s390/s390-32/stackguard-macros.h: Likewise.
+	* sysdeps/s390/s390-64/stackguard-macros.h: Likewise.
+	* sysdeps/sparc/sparc32/stackguard-macros.h: Likewise.
+	* sysdeps/sparc/sparc64/stackguard-macros.h: Likewise.
+
+2013-09-23  Hector Marco  <hecmargi@upv.es>
+	    Ismael Ripoll  <iripoll@disca.upv.es>
+	    Carlos O'Donell  <carlos@redhat.com>
+
+	[BZ #15754]
+	* sysdeps/generic/stackguard-macros.h: Define
+	__pointer_chk_guard_local and POINTER_CHK_GUARD.
+	* 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.
+
 2013-09-15  Vinitha Vijayan <vinitha.vijayann@gmail.com>
 
 	[BZ #15859]
diff --git a/NEWS b/NEWS
index b52b52eea7..62c58b2599 100644
--- a/NEWS
+++ b/NEWS
@@ -10,9 +10,15 @@ Version 2.19
 * The following bugs are resolved with this release:
 
   13985, 14155, 14699, 15427, 15522, 15531, 15532, 15640, 15736, 15748,
-  15749, 15797, 15844, 15849, 15855, 15856, 15857, 15859, 15867, 15886,
-  15887, 15890, 15892, 15893, 15895, 15897, 15905, 15909, 15919, 15921,
-  15923, 15939, 15963, 15966.
+  15749, 15754, 15797, 15844, 15849, 15855, 15856, 15857, 15859, 15867,
+  15886, 15887, 15890, 15892, 15893, 15895, 15897, 15905, 15909, 15919,
+  15921, 15923, 15939, 15963, 15966.
+
+* CVE-2013-4788 The pointer guard used for pointer mangling was not
+  initialized for static applications resulting in the security feature
+  being disabled. The pointer guard is now correctly initialized to a
+  random value for static applications. Existing static applications need
+  to be recompiled to take advantage of the fix (bug 15754).
 
 * CVE-2013-4237 The readdir_r function could write more than NAME_MAX bytes
   to the d_name member of struct dirent, or omit the terminating NUL
diff --git a/csu/libc-start.c b/csu/libc-start.c
index e5da3efd06..c898d06b03 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.  */
diff --git a/elf/Makefile b/elf/Makefile
index aaa9534b2e..cb8da9398a 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 0000000000..7aff3b7b5d
--- /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 0000000000..c344a04db1
--- /dev/null
+++ b/elf/tst-ptrguard1.c
@@ -0,0 +1,202 @@
+/* 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
+extern uintptr_t __pointer_chk_guard;
+# define POINTER_CHK_GUARD __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 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 values are not randomized enough");
+      puts ("nor equal to the default 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/ports/ChangeLog.ia64 b/ports/ChangeLog.ia64
index efe352e6bb..272f73a1b3 100644
--- a/ports/ChangeLog.ia64
+++ b/ports/ChangeLog.ia64
@@ -1,3 +1,8 @@
+2013-09-22  Carlos O'Donell  <carlos@redhat.com>
+
+	[BZ #15754]
+	* sysdeps/ia64/stackguard-macros.h: Define POINTER_CHK_GUARD.
+
 2013-08-30   Ondřej Bílka  <neleai@seznam.cz>
 
 	* sysdeps/ia64/fpu/libm_reduce.S: Fix typos.
diff --git a/ports/ChangeLog.tile b/ports/ChangeLog.tile
index 79d15b464c..dc2e7e412d 100644
--- a/ports/ChangeLog.tile
+++ b/ports/ChangeLog.tile
@@ -1,3 +1,8 @@
+2013-09-22  Carlos O'Donell  <carlos@redhat.com>
+
+	[BZ #15754]
+	* sysdeps/tile/stackguard-macros.h: Define POINTER_CHK_GUARD.
+
 2013-09-20  Andreas Schwab  <schwab@linux-m68k.org>
 
 	* sysdeps/tile/ffs.c (__ffs): Define as hidden.
diff --git a/ports/sysdeps/ia64/stackguard-macros.h b/ports/sysdeps/ia64/stackguard-macros.h
index dc683c28c5..390729327a 100644
--- a/ports/sysdeps/ia64/stackguard-macros.h
+++ b/ports/sysdeps/ia64/stackguard-macros.h
@@ -2,3 +2,6 @@
 
 #define STACK_CHK_GUARD \
   ({ uintptr_t x; asm ("adds %0 = -8, r13;; ld8 %0 = [%0]" : "=r" (x)); x; })
+
+#define POINTER_CHK_GUARD \
+  ({ uintptr_t x; asm ("adds %0 = -16, r13;; ld8 %0 = [%0]" : "=r" (x)); x; })
diff --git a/ports/sysdeps/tile/stackguard-macros.h b/ports/sysdeps/tile/stackguard-macros.h
index 589ea2b0d8..f2e041b99b 100644
--- a/ports/sysdeps/tile/stackguard-macros.h
+++ b/ports/sysdeps/tile/stackguard-macros.h
@@ -4,11 +4,17 @@
 # if __WORDSIZE == 64
 #  define STACK_CHK_GUARD \
   ({ uintptr_t x; asm ("addi %0, tp, -16; ld %0, %0" : "=r" (x)); x; })
+#  define POINTER_CHK_GUARD \
+  ({ uintptr_t x; asm ("addi %0, tp, -24; ld %0, %0" : "=r" (x)); x; })
 # else
 #  define STACK_CHK_GUARD \
   ({ uintptr_t x; asm ("addi %0, tp, -8; ld4s %0, %0" : "=r" (x)); x; })
+#  define POINTER_CHK_GUARD \
+  ({ uintptr_t x; asm ("addi %0, tp, -12; ld4s %0, %0" : "=r" (x)); x; })
 # endif
 #else
 # define STACK_CHK_GUARD \
   ({ uintptr_t x; asm ("addi %0, tp, -8; lw %0, %0" : "=r" (x)); x; })
+# define POINTER_CHK_GUARD \
+  ({ uintptr_t x; asm ("addi %0, tp, -12; lw %0, %0" : "=r" (x)); x; })
 #endif
diff --git a/sysdeps/generic/stackguard-macros.h b/sysdeps/generic/stackguard-macros.h
index ababf65d37..4fa3d96f79 100644
--- a/sysdeps/generic/stackguard-macros.h
+++ b/sysdeps/generic/stackguard-macros.h
@@ -2,3 +2,6 @@
 
 extern uintptr_t __stack_chk_guard;
 #define STACK_CHK_GUARD __stack_chk_guard
+
+extern uintptr_t __pointer_chk_guard_local;
+#define POINTER_CHK_GUARD __pointer_chk_guard_local
diff --git a/sysdeps/i386/stackguard-macros.h b/sysdeps/i386/stackguard-macros.h
index 8c31e197e1..039762927c 100644
--- a/sysdeps/i386/stackguard-macros.h
+++ b/sysdeps/i386/stackguard-macros.h
@@ -2,3 +2,11 @@
 
 #define STACK_CHK_GUARD \
   ({ uintptr_t x; asm ("movl %%gs:0x14, %0" : "=r" (x)); x; })
+
+#define POINTER_CHK_GUARD \
+  ({							\
+     uintptr_t x;					\
+     asm ("movl %%gs:%c1, %0" : "=r" (x)		\
+	  : "i" (offsetof (tcbhead_t, pointer_guard)));	\
+     x;							\
+   })
diff --git a/sysdeps/powerpc/powerpc32/stackguard-macros.h b/sysdeps/powerpc/powerpc32/stackguard-macros.h
index 839f6a4b9b..b3d0af830f 100644
--- a/sysdeps/powerpc/powerpc32/stackguard-macros.h
+++ b/sysdeps/powerpc/powerpc32/stackguard-macros.h
@@ -2,3 +2,13 @@
 
 #define STACK_CHK_GUARD \
   ({ uintptr_t x; asm ("lwz %0,-28680(2)" : "=r" (x)); x; })
+
+#define POINTER_CHK_GUARD \
+  ({												\
+     uintptr_t x;										\
+     asm ("lwz %0,%1(2)"									\
+	  : "=r" (x)										\
+	  : "i" (offsetof (tcbhead_t, pointer_guard) - TLS_TCB_OFFSET - sizeof (tcbhead_t))	\
+         );											\
+     x;												\
+   })
diff --git a/sysdeps/powerpc/powerpc64/stackguard-macros.h b/sysdeps/powerpc/powerpc64/stackguard-macros.h
index 9da879c611..4620f96b86 100644
--- a/sysdeps/powerpc/powerpc64/stackguard-macros.h
+++ b/sysdeps/powerpc/powerpc64/stackguard-macros.h
@@ -2,3 +2,13 @@
 
 #define STACK_CHK_GUARD \
   ({ uintptr_t x; asm ("ld %0,-28688(13)" : "=r" (x)); x; })
+
+#define POINTER_CHK_GUARD \
+  ({												\
+     uintptr_t x;										\
+     asm ("ld %0,%1(2)"										\
+	  : "=r" (x)										\
+	  : "i" (offsetof (tcbhead_t, pointer_guard) - TLS_TCB_OFFSET - sizeof (tcbhead_t))	\
+         );											\
+     x;												\
+   })
diff --git a/sysdeps/s390/s390-32/stackguard-macros.h b/sysdeps/s390/s390-32/stackguard-macros.h
index b74c5799b3..449e8d488f 100644
--- a/sysdeps/s390/s390-32/stackguard-macros.h
+++ b/sysdeps/s390/s390-32/stackguard-macros.h
@@ -2,3 +2,14 @@
 
 #define STACK_CHK_GUARD \
   ({ uintptr_t x; asm ("ear %0,%%a0; l %0,0x14(%0)" : "=a" (x)); x; })
+
+/* On s390/s390x there is no unique pointer guard, instead we use the
+   same value as the stack guard.  */
+#define POINTER_CHK_GUARD \
+  ({							\
+     uintptr_t x;					\
+     asm ("ear %0,%%a0; l %0,%1(%0)"			\
+	  : "=a" (x)					\
+	  : "i" (offsetof (tcbhead_t, stack_guard)));	\
+     x;							\
+   })
diff --git a/sysdeps/s390/s390-64/stackguard-macros.h b/sysdeps/s390/s390-64/stackguard-macros.h
index 0cebb5f022..c8270fbe79 100644
--- a/sysdeps/s390/s390-64/stackguard-macros.h
+++ b/sysdeps/s390/s390-64/stackguard-macros.h
@@ -2,3 +2,17 @@
 
 #define STACK_CHK_GUARD \
   ({ uintptr_t x; asm ("ear %0,%%a0; sllg %0,%0,32; ear %0,%%a1; lg %0,0x28(%0)" : "=a" (x)); x; })
+
+/* On s390/s390x there is no unique pointer guard, instead we use the
+   same value as the stack guard.  */
+#define POINTER_CHK_GUARD \
+  ({							\
+     uintptr_t x;					\
+     asm ("ear %0,%%a0;"				\
+	  "sllg %0,%0,32;"				\
+	  "ear %0,%%a1;"				\
+	  "lg %0,%1(%0)"				\
+	 : "=a" (x)					\
+	 : "i" (offsetof (tcbhead_t, stack_guard)));	\
+     x;							\
+   })
diff --git a/sysdeps/sparc/sparc32/stackguard-macros.h b/sysdeps/sparc/sparc32/stackguard-macros.h
index c0b02b0bbd..1eef0f19f0 100644
--- a/sysdeps/sparc/sparc32/stackguard-macros.h
+++ b/sysdeps/sparc/sparc32/stackguard-macros.h
@@ -2,3 +2,6 @@
 
 #define STACK_CHK_GUARD \
   ({ uintptr_t x; asm ("ld [%%g7+0x14], %0" : "=r" (x)); x; })
+
+#define POINTER_CHK_GUARD \
+  ({ uintptr_t x; asm ("ld [%%g7+0x18], %0" : "=r" (x)); x; })
diff --git a/sysdeps/sparc/sparc64/stackguard-macros.h b/sysdeps/sparc/sparc64/stackguard-macros.h
index 80f063558a..cc0c12c041 100644
--- a/sysdeps/sparc/sparc64/stackguard-macros.h
+++ b/sysdeps/sparc/sparc64/stackguard-macros.h
@@ -2,3 +2,6 @@
 
 #define STACK_CHK_GUARD \
   ({ uintptr_t x; asm ("ldx [%%g7+0x28], %0" : "=r" (x)); x; })
+
+#define POINTER_CHK_GUARD \
+  ({ uintptr_t x; asm ("ldx [%%g7+0x30], %0" : "=r" (x)); x; })
diff --git a/sysdeps/x86_64/stackguard-macros.h b/sysdeps/x86_64/stackguard-macros.h
index d7fedb3737..1948800cd0 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; })