Bug#656656: Please enabled hardened build flags

Kees Cook kees at debian.org
Fri Jan 27 20:01:05 UTC 2012


Hi,

On Fri, Jan 27, 2012 at 07:20:46PM +0100, Moritz Mühlenhoff wrote:
> On Fri, Jan 27, 2012 at 10:00:53AM -0800, Russ Allbery wrote:
> > Russ Allbery <rra at debian.org> writes:
> > > "Cantor, Scott" <cantor.2 at osu.edu> writes:
> > 
> > >> Not that it's necessarily likely here, but with the --silent flag on to
> > >> limit noise, you actually can't tell what the actual compiler command
> > >> is.  There are libtool bugs, usually on Solaris one finds, that break
> > >> the use of some flags. I guess it's possible something like that could
> > >> be happening.
> > 
> > > True.  Okay, let me go do a manual build where I can remove --silent and
> > > be sure that things are actually being passed down to the compiler.
> > 
> > Without --silent, libtool definitely claims to be sending that flag:
> > 
> > /bin/bash ../libtool --tag=CXX   --mode=compile g++ -DHAVE_CONFIG_H -I. -I..   -pthread -g -Wall -O2 -O2 -DNDEBUG -D_FORTIFY_SOURCE=2    -pthread -Wall -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Wformat-security -Werror=format-security -O2 -DNDEBUG -c -o AbstractComplexElement.lo AbstractComplexElement.cpp
> > libtool: compile:  g++ -DHAVE_CONFIG_H -I. -I.. -pthread -g -Wall -O2 -O2 -DNDEBUG -D_FORTIFY_SOURCE=2 -pthread -Wall -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Wformat-security -Werror=format-security -O2 -DNDEBUG -c AbstractComplexElement.cpp  -fPIC -DPIC -o .libs/AbstractComplexElement.o
> > 
> > and I suspended the build in the middle of compiling a source file, and
> > that flag is there in the process arguments:
> > 
> > eagle     9987  0.0  0.0   2088   512 pts/10   T    09:54   0:00 g++ -DHAVE_CONFIG_H -I. -I.. -pthread -g -Wall -O2 -O2 -DNDEBUG -D_FORTIFY_SOURCE=2 -pthread -Wall -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Wformat-security -Werror=format-security -O2 -DNDEBUG -c AbstractComplexElement.cpp -fPIC -DPIC -o .libs/AbstractComplexElement.o
> > 
> > but hardening-check returns the same result:
> > 
> > windlord:~/dvl/debian/xmltooling> hardening-check xmltooling/.libs/libxmltooling.so
> > xmltooling/.libs/libxmltooling.so:
> >  Position Independent Executable: no, regular shared library (ignored)
> >  Stack protected: yes
> >  Fortify Source functions: no, no protected functions found!
> >  Read-only relocations: yes
> >  Immediate binding: no not found!
> > 
> > so if there's a failure here, it seems to be somewhere inside g++, or a
> > need to include more than just -D_FORTIFY_SOURCE=2 to enable this.
> 
> Hmm, I'm not sure what's wrong here.

First of all, in debian/rules:

  # Enable compiler hardening flags.
  export DEB_BUILD_MAINT_OPTIONS = all

Was this intended to be:

  export DEB_BUILD_MAINT_OPTIONS = hardening=all

This may cause trouble with the .so's -fPIC bits, so you can probably leave the entire
line off, unless you want to enable bindnow:

  export DEB_BUILD_MAINT_OPTIONS = hardening=+bindnow

> I'm adding Kees Cook to CC. Kees, did you see similar issues with C++
> on Ubuntu when g++ was patched to use FORTIFY_SOURCE by default?
> This is http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=656656

Looks like there are a few things happening here.

First, there seems to be a bug in hardening-check, which it doesn't notice some
functions:

$ readelf -sW /lib/x86_64-linux-gnu/libc.so.6 | grep memcpy
   327: 00000000000919b0     9 FUNC    WEAK   DEFAULT   12 wmemcpy@@GLIBC_2.2.5
   985: 00000000000f9990    27 FUNC    GLOBAL DEFAULT   12 __wmemcpy_chk@@GLIBC_2.4
  1097: 000000000008ab10    60 IFUNC   GLOBAL DEFAULT   12 memcpy@@GLIBC_2.2.5
  1584: 00000000000f61a0    60 IFUNC   GLOBAL DEFAULT   12 __memcpy_chk@@GLIBC_2.3.4

I'll get this fixed (FUNC vs IFUNC).

However, as pointed out earlier in the bug, raw "memcpy()" is still visible. This is,
ultimately, because the code is performing a check that neither the compile-time nor
run-time code knows how to deal with (i.e. a dynamically sized destination). In this
case (and in the case of being always safe at compile-time), the macros end up just
using memcpy() directly:


#ifndef _GNU_SOURCE
# define _GNU_SOURCE
#endif
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <stdint.h>
#include <string.h>
#include <stdint.h>
#include <stddef.h>
#include <inttypes.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

/* generates inlined memcpy */
void compile_time_safe_and_inlined(int argc, char * argv[])
{
    char buf[80];
    memcpy(buf, argv[1], 40);
    puts(buf);
}

/* generates memcpy() call */
void compile_time_safe(int argc, char * argv[])
{
    char *buf;
    buf = (char *)malloc(16384);
    memcpy(buf, argv[1], 9000);
    puts(buf);
}

/* throws compile-time warning, generates __memcpy_chk() call */
void compile_time_unsafe(int argc, char * argv[])
{
    char *buf;
    buf = (char *)malloc(16384);
    memcpy(buf, argv[1], 90000);
    puts(buf);
}

/* generates __memcpy_chk() call */
void runtime_verifiable(int argc, char * argv[])
{
    char *buf;
    buf = (char *)malloc(1024);
    memcpy(buf, argv[1], atoi(argv[2]));
    puts(buf);
}

/* generates memcpy() call */
void runtime_unverifiable(int argc, char * argv[])
{
    char *buf;
    buf = (char *)malloc(atoi(argv[3]));
    memcpy(buf, argv[1], atoi(argv[2]));
    puts(buf);
}

int main(int argc, char * argv[])
{
    compile_time_safe_and_inlined(argc, argv);
    compile_time_safe(argc, argv);
    compile_time_unsafe(argc, argv);
    runtime_verifiable(argc, argv);
    runtime_unverifiable(argc, argv);

    return 0;
}

$ gcc -Wall -O2 -D_FORTIFY_SOURCE=2 -o test test.c
$ ./hardening-check --verbose test
test:
 Position Independent Executable: no, normal executable!
 Stack protected: yes
 Fortify Source functions: yes (some protected functions found)
        unprotected: memcpy
        protected: memcpy
 Read-only relocations: yes
 Immediate binding: no not found!

(note the above is using the fixed hardening-check)


The trouble here is that there's no good way I know of to determine which kind of
decision was made at build-time about some of the maybe-protected functions, which
leads us to these kinds of false alarms.

To me, outside of the hardening-check bug that I'll go fix, this is a false alarm.

-Kees

-- 
Kees Cook                                            @debian.org





More information about the Pkg-shibboleth-devel mailing list