Package: eglibc
Version: 2.10.2-3
Severity: normal
Tags: patch
User: ubuntu-de...@lists.ubuntu.com
Usertags: origin-ubuntu lucid ubuntu-patch

Hello!

As more packages (perhaps all!) start using either hardening-wrapper
or the hardening-includes packages to gain the -D_FORTIFY_SOURCE=2 and
-fstack-protector compiler flags, it starts becoming important to handle
a number of special cases that upstream glibc either hasn't acted on or
has inappropriately rejected.

I would like to include the following patches that Ubuntu has carried for
several releases now.  (Note that submitted-leading-zero-stack-guard.diff
will need to be adjusted slightly if stack-guard-quick-randomization.diff
is not applied.)

no-sprintf-pre-truncate.diff
    The sprintf function used when -D_FORTIFY_SOURCE=2 is used incorrectly
    pre-truncates the destination buffer; this changes the long-standing
    expectation of sprintf(foo,"%sbaz",foo) to work.  See the patch for
    further discussion.

local-fwrite-no-attr-unused.diff
    Again, patch contains discussion, but basically, this disables a
    useless and noisy warning that -D_FORTIFY_SOURCE=2 triggers.

stack-guard-quick-randomization.diff
    For applications built with -fstack-protector, this adds the
    "poor man's" randomization for when AT_RANDOM is not available.
    Since AT_RANDOM is in 2.6.31 and later, it may not be useful to
    carry any longer and may not be kfreebsd friendly.

submitted-leading-zero-stack-guard.diff
    This is important as the recent glibc fails to keep the first byte
    of the stack guard a NULL when constructing the stack guard for use
    with -fstack-protector.  Without this, it is possible to potentially
    read and write beyond the stack guard value using NULL-terminated
    string overflows.

Thanks!

-Kees

-- 
Kees Cook                                            @debian.org
Description: when a program is compiled with -D_FORTIFY_SOURCE=2, the
 vsprintf_chk function is called to handle sprintf/snprintf, but it
 needlessly pretruncates the destination which changes the results of
 sprintf(foo, "%sbar", baz).
Bug: http://sourceware.org/bugzilla/show_bug.cgi?id=7075
Bug-Ubuntu: https://launchpad.net/bugs/305901
Author: Kees Cook <kees.c...@canonical.com>

Index: glibc-2.9/debug/vsprintf_chk.c
===================================================================
--- glibc-2.9.orig/debug/vsprintf_chk.c	2008-12-23 21:30:07.000000000 -0800
+++ glibc-2.9/debug/vsprintf_chk.c	2008-12-23 21:30:19.000000000 -0800
@@ -76,7 +76,6 @@
 
   _IO_no_init (&f._sbf._f, _IO_USER_LOCK, -1, NULL, NULL);
   _IO_JUMPS (&f._sbf) = &_IO_str_chk_jumps;
-  s[0] = '\0';
   _IO_str_init_static_internal (&f, s, slen - 1, s);
 
   /* For flags > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
Description: when compiling with -D_FORTIFY_SOURCE=2, the compiler will
 generate warn-unused-results notifications for several functions.  It
 is not sensible to do this for fwrite() since it is frequently unchecked
 and may not fail until fclose() which is not marked with __wur, making
 the fwrite() check noisy and pointless.
Author: Matthias Klose <d...@ubuntu.com>

--- ./libio/stdio.h~	2008-05-24 20:14:36.000000000 +0200
+++ ./libio/stdio.h	2009-03-27 20:59:20.000000000 +0100
@@ -682,7 +682,7 @@
    This function is a possible cancellation points and therefore not
    marked with __THROW.  */
 extern size_t fwrite (__const void *__restrict __ptr, size_t __size,
-		      size_t __n, FILE *__restrict __s) __wur;
+		      size_t __n, FILE *__restrict __s);
 __END_NAMESPACE_STD
 
 #ifdef __USE_GNU
@@ -706,7 +706,7 @@
 extern size_t fread_unlocked (void *__restrict __ptr, size_t __size,
 			      size_t __n, FILE *__restrict __stream) __wur;
 extern size_t fwrite_unlocked (__const void *__restrict __ptr, size_t __size,
-			       size_t __n, FILE *__restrict __stream) __wur;
+			       size_t __n, FILE *__restrict __stream);
 #endif
 
 
Description: when AT_RANDOM is not available, attempt to build randomization
 of stack guard value from the ASLR of stack and heap locations, and finally
 the hp_timing_t value.  Upstream glibc does not want this patch, as they
 feel AT_RANDOM is sufficient.
Author: Jakub Jelinek
Origin: http://cvs.fedora.redhat.com/viewvc/devel/glibc/
Forwarded: not-needed

---
 elf/tst-stackguard1.c               |    8 ++++++--
 nptl/tst-stackguard1.c              |    8 ++++++--
 sysdeps/unix/sysv/linux/dl-osinfo.h |   29 +++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 4 deletions(-)

Index: b/sysdeps/unix/sysv/linux/dl-osinfo.h
===================================================================
--- a/sysdeps/unix/sysv/linux/dl-osinfo.h
+++ b/sysdeps/unix/sysv/linux/dl-osinfo.h
@@ -17,10 +17,13 @@
    Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
    02111-1307 USA.  */
 
+#include <errno.h>
 #include <kernel-features.h>
 #include <dl-sysdep.h>
 #include <fcntl.h>
 #include <stdint.h>
+#include <hp-timing.h>
+#include <endian.h>
 
 #ifndef MIN
 # define MIN(a,b) (((a)<(b))?(a):(b))
@@ -80,6 +83,32 @@
       unsigned char *p = (unsigned char *) &ret;
       p[sizeof (ret) - 1] = 255;
       p[sizeof (ret) - 2] = '\n';
+#ifdef HP_TIMING_NOW
+      hp_timing_t hpt;
+      HP_TIMING_NOW (hpt);
+      hpt = (hpt & 0xffff) << 8;
+      ret ^= hpt;
+#endif
+      uintptr_t stk;
+      /* Avoid GCC being too smart.  */
+      asm ("" : "=r" (stk) : "r" (p));
+      stk &= 0x7ffff0;
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+      stk <<= (__WORDSIZE - 23);
+#elif __WORDSIZE == 64
+      stk <<= 31;
+#endif
+      ret ^= stk;
+      /* Avoid GCC being too smart.  */
+      p = (unsigned char *) &errno;
+      asm ("" : "=r" (stk) : "r" (p));
+      stk &= 0x7fff00;
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+      stk <<= (__WORDSIZE - 29);
+#else
+      stk >>= 8;
+#endif
+      ret ^= stk;
     }
   else
 #endif
Index: b/elf/tst-stackguard1.c
===================================================================
--- a/elf/tst-stackguard1.c
+++ b/elf/tst-stackguard1.c
@@ -160,17 +160,21 @@
      the 16 runs, something is very wrong.  */
   int ndifferences = 0;
   int ndefaults = 0;
+  int npartlyrandomized = 0;
   for (i = 0; i < N; ++i) 
     {
       if (child_stack_chk_guards[i] != child_stack_chk_guards[i+1])
 	ndifferences++;
       else if (child_stack_chk_guards[i] == default_guard)
 	ndefaults++;
+      else if (*(char *) &child_stack_chk_guards[i] == 0)
+	npartlyrandomized++;
     }
 
-  printf ("differences %d defaults %d\n", ndifferences, ndefaults);
+  printf ("differences %d defaults %d partly randomized %d\n",
+	  ndifferences, ndefaults, npartlyrandomized);
 
-  if (ndifferences < N / 2 && ndefaults < N / 2)
+  if ((ndifferences + ndefaults + npartlyrandomized) < 3 * N / 4)
     {
       puts ("stack guard canaries are not randomized enough");
       puts ("nor equal to the default canary value");
Index: b/nptl/tst-stackguard1.c
===================================================================
--- a/nptl/tst-stackguard1.c
+++ b/nptl/tst-stackguard1.c
@@ -190,17 +190,21 @@
      the 16 runs, something is very wrong.  */
   int ndifferences = 0;
   int ndefaults = 0;
+  int npartlyrandomized = 0;
   for (i = 0; i < N; ++i) 
     {
       if (child_stack_chk_guards[i] != child_stack_chk_guards[i+1])
 	ndifferences++;
       else if (child_stack_chk_guards[i] == default_guard)
 	ndefaults++;
+      else if (*(char *) &child_stack_chk_guards[i] == 0)
+	npartlyrandomized++;
     }
 
-  printf ("differences %d defaults %d\n", ndifferences, ndefaults);
+  printf ("differences %d defaults %d partly randomized %d\n",
+	  ndifferences, ndefaults, npartlyrandomized);
 
-  if (ndifferences < N / 2 && ndefaults < N / 2)
+  if ((ndifferences + ndefaults + npartlyrandomized) < 3 * N / 4)
     {
       puts ("stack guard canaries are not randomized enough");
       puts ("nor equal to the default canary value");
Description: require that the first byte in the stack guard in a NULL byte,
 to improve mitigation of NULL-terminated string overflows.
Bug: http://sourceware.org/bugzilla/show_bug.cgi?id=10149
Bug-Ubuntu: https://bugs.launchpad.net/bugs/413278
Author: Kees Cook <kees.c...@canonical.com>

Index: eglibc-2.10.1/sysdeps/unix/sysv/linux/dl-osinfo.h
===================================================================
--- eglibc-2.10.1.orig/sysdeps/unix/sysv/linux/dl-osinfo.h	2009-08-12 16:30:26.000000000 -0700
+++ eglibc-2.10.1/sysdeps/unix/sysv/linux/dl-osinfo.h	2009-08-12 16:32:55.000000000 -0700
@@ -65,7 +65,12 @@
 static inline uintptr_t __attribute__ ((always_inline))
 _dl_setup_stack_chk_guard (void *dl_random)
 {
-  uintptr_t ret;
+  uintptr_t ret = 0;
+  /* Having a leading zero byte protects the stack guard from being
+     overwritten with str* write operations or exposed by an
+     unterminated str* read operation. */
+  unsigned char *p = ((unsigned char *) &ret) + 1;
+  int size = sizeof (ret) - 1;
 #ifndef __ASSUME_AT_RANDOM
   if (__builtin_expect (dl_random == NULL, 0))
     {
@@ -73,16 +78,16 @@
       int fd = __open ("/dev/urandom", O_RDONLY);
       if (fd >= 0)
 	{
-	  ssize_t reslen = __read (fd, &ret, sizeof (ret));
+	  ssize_t reslen = __read (fd, p, size);
 	  __close (fd);
-	  if (reslen == (ssize_t) sizeof (ret))
+	  if (reslen == (ssize_t) size)
 	    return ret;
 	}
 # endif
-      ret = 0;
-      unsigned char *p = (unsigned char *) &ret;
-      p[sizeof (ret) - 1] = 255;
-      p[sizeof (ret) - 2] = '\n';
+      /* Lacking any other form of randomized stack guard, add other
+         terminators in an attempt to block things like fgets, etc. */
+      p[size - 1] = 255;
+      p[size - 2] = '\n';
 #ifdef HP_TIMING_NOW
       hp_timing_t hpt;
       HP_TIMING_NOW (hpt);
@@ -115,7 +120,7 @@
     /* We need in the moment only 8 bytes on 32-bit platforms and 16
        bytes on 64-bit platforms.  Therefore we can use the data
        directly and not use the kernel-provided data to seed a PRNG.  */
-    memcpy (&ret, dl_random, sizeof (ret));
+    memcpy (p, dl_random, size);
   return ret;
 }
 

Reply via email to