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; }