disabling FORTIFY_SOURCE

[Kees Cook]

Hi,

It seems to me that disabling FORTIFY_SOURCE is a mistake. It offers
a great many protections, and virtually every distribution has very
intentionally turned on this compiler flag by default. Given Wine's
size[1], I would argue the benefits[2] outweigh the hassle of rearranging
the structures and accessors to not trick the compiler into allocating
memory beyond the end of the structure for incoming strings.

It has found, at least in other projects, a lot of potential problems,
and better yet, has repeatedly turned exploitable vulnerabilities into
simple denial of services. I realize it's a bit of a pain to work with
given how you're building some of the structures, but I'd like to ask
that it not be globally disabled.

Thanks,

-Kees

[1] $ find . -type f -name '*.[ch]' | xargs wc -l | grep total
     2678911 total

[2] Some details at https://wiki.ubuntu.com/CompilerFlags#-D_FORTIFY_SOURCE=2
    but at least the following...

Compile time:
    - static buffer length checks
    - missed return values
    - open() checks for missing mode when used with O_CREAT

Run time:
    - dynamic buffer length checks
    - dynamic format string safety check
    - dynamic format position safety checks

Functions with buffer length (or other) checks:

asprintf confstr dprintf fgets fgetws fprintf fread fwprintf getcwd
    getdomainname getgroups gethostname getlogin_r gets getwd longjmp
    mbsnrtowcs mbsrtowcs mbstowcs memcpy memmove mempcpy memset pread64 pread
    printf ptsname_r read readlinkat readlink realpath recv recvfrom snprintf
    sprintf stpcpy stpncpy strcat strcpy strncat strncpy swprintf syslog
    ttyname_r vasprintf vdprintf vfprintf vfwprintf vprintf vsnprintf vsprintf
    vswprintf vsyslog vwprintf wcpcpy wcpncpy wcrtomb wcscat wcscpy wcsncat
    wcsncpy wcsnrtombs wcsrtombs wcstombs wctomb wmemcpy wmemmove wmempcpy
    wmemset wprintf

[Alexandre Julliard]

Kees Cook  writes:
> It seems to me that disabling FORTIFY_SOURCE is a mistake. It offers
> a great many protections, and virtually every distribution has very
> intentionally turned on this compiler flag by default. Given Wine's
> size[1], I would argue the benefits[2] outweigh the hassle of rearranging
> the structures and accessors to not trick the compiler into allocating
> memory beyond the end of the structure for incoming strings.
>
> It has found, at least in other projects, a lot of potential problems,
> and better yet, has repeatedly turned exploitable vulnerabilities into
> simple denial of services.

So far in Wine, all it has done is repeatedly turn perfectly valid code
into denial of service.

Actually, even if Fortify worked correctly, the benefits would most
likely be small, given that we make little use of the standard libc
functions. Though given the trouble we've had so far, I shudder to think
what would happen if we used libc functions all over the place.

Fortify is a nice idea in theory, and I'd certainly encourage developers
to enable it to see if it catches anything useful. But at this point
it's not reliable enough to be forced upon end users.