8:52 a.m.
Hi all,
In order for FORTIFY_SOURCE [1] to take effect there are two
requirements: 1.) use correct optflags as defined by the guidelines [2],
and 2.) include header files when using function that can potentially be
fortified by replacing with macros.
[1] http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02055.html
[2]
http://fedoraproject.org/wiki/Packaging/Guidelines#head-8b14098227aebff1c...
The second problem is not specifically FORTIFY_SOURCE related; you
should really include respective headers, many routines can really be
macros; such as open(). POSIX allows this iirc.
Now -- your action is needed. Please look at the list [3] to see if your
package passed a simple test [4]:
[3] http://people.redhat.com/lkundrak/check-fortify/
[4] https://bugzilla.redhat.com/attachment.cgi?id=298147
If not, do the following:
1.) See [5] for rough explanation of the problem and what can you do
with that
2.) Search the build.log from your latest build for calls of "gcc"
without the typical optflags (look for -DFORTIFY_SOURCE=2). In case you
see those you don't pass the compiler flags correctly. Examples: [6] [7]
3.) If you see warnings about implicit declarations of functions in
build.log, the application omitted some header files.
If you have more time, try to rebuild your package with
"-Werror-implicit-function-declaration" added to %optflags in your
~/.rpmmacros to see if you rely on implicit declarations where you
should include the header files with macros and/or prototypes instead.
[5] http://ovecka.be/~lkundrak/blog/entries/fortify-check.html
[6]
http://koji.fedoraproject.org/packages/hal-cups-utils/0.6.15/1.fc9/data/l...
[7] http://koji.fedoraproject.org/packages/iptraf/3.0.1/3.fc9/data/logs/i386/...
Should you find any errors, or have comments or questions, don't
hesitate to mail me.
Thanks,
--
Lubomir Kundrak (Red Hat Security Response Team)
8:57 a.m.
Isn't this something which should be in rpmlint?
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
9:13 a.m.
Josh Boyer wrote:
On Thu, 20 Mar 2008 13:57:48 +0000
"Richard W.M. Jones" <rjones(a)redhat.com> wrote:
> Isn't this something which should be in rpmlint?
No? Because it can't catch the header includes... It's an rpm lint
checker, not a source scanner.
However this could be added to Makefile.common, it would be simple to check
this after a make %arch using the generated .build-fooo.log
Regards,
Hans
9:44 a.m.
Hi Hans, Josh and Richard,
On Thu, 2008-03-20 at 15:13 +0100, Hans de Goede wrote:
Josh Boyer wrote:
> On Thu, 20 Mar 2008 13:57:48 +0000
> "Richard W.M. Jones" <rjones(a)redhat.com> wrote:
>
>> Isn't this something which should be in rpmlint?
>
> No? Because it can't catch the header includes... It's an rpm lint
> checker, not a source scanner.
Surely enough rpmlint can look for offensive symbols; it already
contains checks that look into file content (end-of-line encoding check,
or shebang in scripts check.).
On the other hand there are rpm post install checks that do the similar
thing -- consider the one that checks for rpaths.
I am wondering if they can be merged into rpmlint, and the rpmlint
changed into differentiating between warnings and errors, so it itself
can be invoked as post-install check? Anybody has an opinion on this? Is
this worth filing a rpmlint feature request?
However this could be added to Makefile.common, it would be simple to
check
this after a make %arch using the generated .build-fooo.log
I am trying to get the script in rpm-build package:
https://bugzilla.redhat.com/show_bug.cgi?id=437627
Then we can add it to default rpmdevtools' rpmmacros.
Then the next steps would be adding making it a mandatory post install
check, what would output warnings, and then cause the builds to fail.
And then we can make "-Werror-implicit-function-declaration" a default
optflag, and whitelist applications that will have to use the old
optflags to build. And then get rid of that whitelist.
But these steps will need to be approved by packaging commitee, or maybe
by FESCO, and I guess they'll not get approvals if they break a big
number of packages. Therefore each step needs some time to reduce number
of broken packages. This is just the first step, to get the idea of
number of packages, and also to clean up the most obvious violators, so
the change proposals have bigger chance to pass once they're written and
presented to respective commitees.
Thanks,
--
Lubomir Kundrak (Red Hat Security Response Team)
9:57 a.m.
OK, so one of my packages shows up on this list. But I've verified
that the compiler is indeed called with the proper flags in all cases,
there are no instances of implicit declarations of anything (no lines
matching "implicit" or "declaration" in the build log), as far as I
can tell, the code does not define the problematic function (sprintf)
itself, and the hostname in the URL
http://ovecka.be/~lkundrak/blog/entries/fortify-check.html doesn't
resolve.
So what's to be done? My understanding was that we'd try to pass
these flags at all times but that there's no strict guarantee that
they will actually function on any particular piece of code and that
we shouldn't go rewriting upstream code to make them work when there
is no security exposure (as in the case of my package).
- J<
10:56 a.m.
On Thu, 2008-03-20 at 09:57 -0500, Jason L Tibbitts III wrote:
OK, so one of my packages shows up on this list. But I've
verified
that the compiler is indeed called with the proper flags in all cases,
there are no instances of implicit declarations of anything (no lines
matching "implicit" or "declaration" in the build log), as far as I
can tell, the code does not define the problematic function (sprintf)
itself, and the hostname in the URL
http://ovecka.be/~lkundrak/blog/entries/fortify-check.html doesn't
resolve.
$ find nazghul -name '*.[ch]' |xargs grep -l printf |xargs grep -L
stdio.h
nazghul/src/ascii.c
...
nazghul/config.h
$
These files do use *printf, but don't include <stdio.h>.
Please patch them and send patch upstream if possible.
So what's to be done? My understanding was that we'd try to
pass
these flags at all times but that there's no strict guarantee that
they will actually function on any particular piece of code and that
we shouldn't go rewriting upstream code to make them work when there
is no security exposure (as in the case of my package).
There's always chance for *printf functions to be used incorrectly and
make up and attack vector for format string attacks. Consider situation
when they are used to output a file name of a randomly named file that's
in their working directory, etc.
Thanks,
--
Lubomir Kundrak (Red Hat Security Response Team)
11:36 a.m.
>>>> "LK" == Lubomir Kundrak
<lkundrak(a)redhat.com> writes:
LK> nazghul/src/ascii.c
LK> nazghul/config.h
LK> These files do use *printf, but don't include <stdio.h>. Please
LK> patch them and send patch upstream if possible.
Well, config.h is generated by autoconf, and I don't believe it
includes any function calls at all. Am I going to have to patch
autoconf now, or is this a false positive?
ascii.c does indeed call printf, but it indirectly includes stdio.h
(via at least SDL_image.h which includes SDL.h which includes
SDL_stdinc.h which includes stdio.h). Perhaps not the best
programming practise, but he compiler doesn't complain that *printf is
used before being defined anywhere in the build log. Wouldn't there
be at least a warning in this case? I don't think it matters (except
for issues of style) where the header is included as long as it's
included previous to the site of the function call.
So I guess I'm still confused.
- J<
10:58 a.m.
Jason L Tibbitts III wrote, at 03/20/2008 11:57 PM +9:00:
OK, so one of my packages shows up on this list. But I've
verified
that the compiler is indeed called with the proper flags in all cases,
there are no instances of implicit declarations of anything (no lines
matching "implicit" or "declaration" in the build log), as far as I
can tell, the code does not define the problematic function (sprintf)
itself, and the hostname in the URL
http://ovecka.be/~lkundrak/blog/entries/fortify-check.html doesn't
resolve.
Also I have no idea about my package qdbm and hyperestraier.
Regards,
Mamoru
11:21 a.m.
On Fri, 2008-03-21 at 00:58 +0900, Mamoru Tasaka wrote:
Jason L Tibbitts III wrote, at 03/20/2008 11:57 PM +9:00:
> OK, so one of my packages shows up on this list. But I've verified
> that the compiler is indeed called with the proper flags in all cases,
> there are no instances of implicit declarations of anything (no lines
> matching "implicit" or "declaration" in the build log), as far
as I
> can tell, the code does not define the problematic function (sprintf)
> itself, and the hostname in the URL
> http://ovecka.be/~lkundrak/blog/entries/fortify-check.html doesn't
> resolve.
Also I have no idea about my package qdbm and hyperestraier.
Mamoru, thanks for telling me, I'll look at it.
Currently I have trouble to find out how did the sprintf symbol get into
tibbs' nazghul binary when none of object symbols it is linked from
refers to it. So don't worry about your packages, it might be the same
case.
To others: If you can't find out why is your package on the list, don't
worry -- just send me the name. In case the number of the packages will
be bigger, then I must have done something wrong when generating the
list -- and in that case it won't be smart to send each package name to
the list. Feel free to mail me privately or via IRC.
Thanks,
--
Lubomir Kundrak (Red Hat Security Response Team)
11:49 a.m.
On Thu, Mar 20, 2008 at 10:21 AM, Lubomir Kundrak <lkundrak(a)redhat.com> wrote:
Currently I have trouble to find out how did the sprintf symbol get
into
tibbs' nazghul binary when none of object symbols it is linked from
refers to it. So don't worry about your packages, it might be the same
case.
To others: If you can't find out why is your package on the list, don't
worry -- just send me the name. In case the number of the packages will
be bigger, then I must have done something wrong when generating the
list -- and in that case it won't be smart to send each package name to
the list. Feel free to mail me privately or via IRC.
Here's some information that might help others. My package jlint is
on the list. It compiles all source files with the standard
%optflags, and the compiler issues no warnings about missing function
declarations. Invoking 'nm -CD /usr/bin/jlint" lists __fprintf_chk,
__printf_chk, __sprintf_chk ... and sprintf. Compiling by hand shows
that all 4 symbols wind up in the jlint.o object file, even before
linking. Compiling with "gcc -E" showed nothing out of the ordinary;
the inline sprintf function that expands to a call to __sprintf_chk is
there, all right, and before any invocations of sprintf. I had to dig
through a disassembly of the object file to find the problem. That
was fun.
So gcc is failing to inline the "always inline" sprintf function in
one place, jlint.cpp line 189. That particular invocation of sprintf
looks just like several others, so I don't know why gcc did not inline
it. Using "-Winline" doesn't help, because it doesn't warn about
inline functions declared in system headers. Using "-Wsystem-headers
-Winline" also prints no warnings, which I consider a gcc bug. I
tried cranking "-finline-limit" up from the default of 600 all the way
to 100000 without convincing gcc to inline that call.
Bottom line:
1) As long as gcc can fail to inline calls, your script is going to
emit false positives.
2) GCC refuses to report that it failed to inline the functions of interest.
--
Jerry James
http://loganjerry.googlepages.com/
2 p.m.
On Fri, Mar 21, 2008 at 10:49 AM, I wrote:
Here's some information that might help others. My package
jlint is
on the list. It compiles all source files with the standard
%optflags, and the compiler issues no warnings about missing function
declarations. Invoking 'nm -CD /usr/bin/jlint" lists __fprintf_chk,
__printf_chk, __sprintf_chk ... and sprintf. Compiling by hand shows
that all 4 symbols wind up in the jlint.o object file, even before
linking. Compiling with "gcc -E" showed nothing out of the ordinary;
the inline sprintf function that expands to a call to __sprintf_chk is
there, all right, and before any invocations of sprintf. I had to dig
through a disassembly of the object file to find the problem. That
was fun.
So gcc is failing to inline the "always inline" sprintf function in
one place, jlint.cpp line 189. That particular invocation of sprintf
looks just like several others, so I don't know why gcc did not inline
it. Using "-Winline" doesn't help, because it doesn't warn about
inline functions declared in system headers. Using "-Wsystem-headers
-Winline" also prints no warnings, which I consider a gcc bug. I
tried cranking "-finline-limit" up from the default of 600 all the way
to 100000 without convincing gcc to inline that call.
Bottom line:
1) As long as gcc can fail to inline calls, your script is going to
emit false positives.
2) GCC refuses to report that it failed to inline the functions of interest.
Why are the functions affected by FORTIFY_SOURCE defined as inline
functions instead of preprocessor macros? In the former case, you can
never be sure that the compiler will inline everywhere. In the latter
case, you know the replacement will happen. Is there some technical
reason for using inline functions?
--
Jerry James
http://loganjerry.googlepages.com/
11:18 a.m.
On Thu, 2008-03-20 at 08:08 -0800, Jeff Spaleta wrote:
On Thu, Mar 20, 2008 at 5:52 AM, Lubomir Kundrak
<lkundrak(a)redhat.com> wrote:
> Should you find any errors, or have comments or questions, don't
> hesitate to mail me.
Are you planning on opening bugs against packages in that list?
Not yet. I'll see if that list can be made shorter without bugs.
--
Lubomir Kundrak (Red Hat Security Response Team)
12:39 p.m.
On Thu, 2008-03-20 at 14:52 +0100, Lubomir Kundrak wrote:
Should you find any errors, or have comments or questions, don't
hesitate to mail me.
cdparanoia appears to be a false positive:
ketamine:/redhat/core/cdparanoia/devel% grep ^gcc .build-alpha9.8-29.log | grep -v
FORTIFY_SOURCE
gcc -fpic -shared -o libcdda_interface.so.0.9.8 -Wl,-soname -Wl,libcdda_interface.so.0
scan_devices.o common_interface.o cooked_interface.o interface.o scsi_interface.o
smallft.o toc.o test_interface.o -lm
gcc -fpic -shared -o libcdda_paranoia.so.0.9.8 -Wl,-soname -Wl,libcdda_paranoia.so.0
paranoia.o p_block.o overlap.o gap.o isort.o -L ../interface -lcdda_interface
Unless I'm wildly mistaken, -DFOO on the command line when invoking gcc
as a fancy frontend to ld should have no effect.
There are some implicit function declarations, which I've fixed, but
memcmp() and exit() aren't fortified functions, and aren't the functions
your test warned about anyway.
- ajax
9:36 a.m.
On Thu March 20 2008, Lubomir Kundrak wrote:
If you have more time, try to rebuild your package with
"-Werror-implicit-function-declaration" added to %optflags in your
~/.rpmmacros to see if you rely on implicit declarations where you
should include the header files with macros and/or prototypes instead.
Would you please post the exact line that needs to be added to .rpmmacros to
achieve this?
I tried
%optflags %{optflags} -Werror-implicit-function-declaration
and
%optflags -Werror-implicit-function-declaration
but both lines do not change the output of
rpm --eval %optflags
or
rpm --eval %configure
Regards,
Till
9:31 a.m.
On Friday 21 March 2008, Till Maas wrote:
On Thu March 20 2008, Lubomir Kundrak wrote:
> If you have more time, try to rebuild your package with
> "-Werror-implicit-function-declaration" added to %optflags in your
> ~/.rpmmacros to see if you rely on implicit declarations where you
> should include the header files with macros and/or prototypes instead.
Would you please post the exact line that needs to be added to .rpmmacros
to achieve this?
I tried
%optflags %{optflags} -Werror-implicit-function-declaration
and
%optflags -Werror-implicit-function-declaration
but both lines do not change the output of
rpm --eval %optflags
or
rpm --eval %configure
optflags are set in rpmrc, and it doesn't seem possible to override them with
rpmmacros. Setting it in ~/.rpmrc works for me. Look up the default for
your architecture from /usr/lib/rpm/redhat/rpmrc, copy-paste that line into
your ~/.rpmrc and add stuff you want to it.
2:49 p.m.
On Sat March 22 2008, Ville Skyttä wrote:
On Friday 21 March 2008, Till Maas wrote:
> On Thu March 20 2008, Lubomir Kundrak wrote:
> > If you have more time, try to rebuild your package with
> > "-Werror-implicit-function-declaration" added to %optflags in your
> > ~/.rpmmacros to see if you rely on implicit declarations where you
> > should include the header files with macros and/or prototypes instead.
>
> Would you please post the exact line that needs to be added to .rpmmacros
> to achieve this?
optflags are set in rpmrc, and it doesn't seem possible to
override them
with rpmmacros. Setting it in ~/.rpmrc works for me. Look up the default
for your architecture from /usr/lib/rpm/redhat/rpmrc, copy-paste that line
into your ~/.rpmrc and add stuff you want to it.
Thank you, this worked. vbetool has been fixed now.
Regards,
Till
6:15 a.m.
On Thu, Mar 20, 2008 at 02:52:40PM +0100, Lubomir Kundrak wrote:
Hi all,
The second problem is not specifically FORTIFY_SOURCE related; you
should really include respective headers, many routines can really be
macros; such as open(). POSIX allows this iirc.
If you have more time, try to rebuild your package with
"-Werror-implicit-function-declaration" added to %optflags in your
~/.rpmmacros to see if you rely on implicit declarations where you
should include the header files with macros and/or prototypes instead.
There are about 200 of such functions in a package I maintain, the
cernlib. Is it really important to fix them? What are the implications?
Though the implicit declarations arer not for printf...
--
Pat
1:11 p.m.
Patrice Dumas wrote:
On Thu, Mar 20, 2008 at 02:52:40PM +0100, Lubomir Kundrak wrote:
> Hi all,
>
> The second problem is not specifically FORTIFY_SOURCE related; you
> should really include respective headers, many routines can really be
> macros; such as open(). POSIX allows this iirc.
>
> If you have more time, try to rebuild your package with
> "-Werror-implicit-function-declaration" added to %optflags in your
> ~/.rpmmacros to see if you rely on implicit declarations where you
> should include the header files with macros and/or prototypes instead.
There are about 200 of such functions in a package I maintain, the
cernlib. Is it really important to fix them? What are the implications?
Though the implicit declarations arer not for printf...
Yes fixing them is important, missing prototypes can lead to all sort of errors
and often lead to 64 bit issues (== crashes).
Maybe we should make -Werror-implicit-function-declaration part of the default
optflags?
Regards,
Hans
1:58 p.m.
On Mon, Mar 24, 2008 at 07:11:27PM +0100, Hans de Goede wrote:
Yes fixing them is important, missing prototypes can lead to all sort of
errors and often lead to 64 bit issues (== crashes).
And more specifically? What kid of 64bit issues and for which function?
For that example, I don't mind fixing dome missing prototypes, but all
is certainly a waste of my time.
--
Pat
3:39 p.m.
Patrice Dumas <pertusus(a)free.fr> writes:
> Yes fixing them is important, missing prototypes can lead to all
sort
> of errors and often lead to 64 bit issues (== crashes).
And more specifically? What kid of 64bit issues and for which
function?
Especially functions with >=7 arguments:
void foo(int a1, int a2, int a3, int a4, int a5, int a6, int *e)
{
if (e)
*e = a1;
}
int main()
{
foo(1,2,3,4,5,6, 0);
}
This will segfault when foo() and main() are in two different compilation
units.
Enrico
4:01 p.m.
Patrice Dumas wrote:
On Mon, Mar 24, 2008 at 07:11:27PM +0100, Hans de Goede wrote:
> Yes fixing them is important, missing prototypes can lead to all sort of
> errors and often lead to 64 bit issues (== crashes).
And more specifically? What kid of 64bit issues and for which function?
For that example, I don't mind fixing dome missing prototypes, but all
is certainly a waste of my time.
Anything (any function) that returns a pointer, will be a problem when compiled
on 64 bit without a prototype, as the compiler will assume the return type is
an int, and then cast that to a pointer as needed.
Anything returning a float / double will be a problem regardless of 32/64 bits
as only 32 bits of the returned floating point number will be taken and
interpreted as an int (and then cast back to a float / double if used as such).
Any function called with arguments of different type then expected (for example
a float, where the function expects an int or visa versa) will have the same
problem.
Which is why I say: fix them all.
Regards,
Hans
12:50 p.m.
Hans de Goede <j.w.r.degoede(a)hhs.nl> wrote:
Patrice Dumas wrote:
> On Mon, Mar 24, 2008 at 07:11:27PM +0100, Hans de Goede wrote:
>> Yes fixing them is important, missing prototypes can lead to all
>> sort of errors and often lead to 64 bit issues (== crashes).
> And more specifically? What kid of 64bit issues and for which
> function?
> For that example, I don't mind fixing dome missing prototypes, but all
> is certainly a waste of my time.
>
Anything (any function) that returns a pointer, will be a problem when
compiled on 64 bit without a prototype, as the compiler will assume
the return type is an int, and then cast that to a pointer as needed.
Anything returning a float / double will be a problem regardless of
32/64 bits as only 32 bits of the returned floating point number will
be taken and interpreted as an int (and then cast back to a float /
double if used as such).
Nope, on i386 doubles are returned in the floating point registers, ints
are returned in EAX. You just get whatever garbage was left over in "the
other one".
Any function called with arguments of different type then expected
(for example a float, where the function expects an int or visa versa)
will have the same problem.
Yep. The "default promotion rules" of C kick in.n
--
Dr. Horst H. von Brand User #22616 counter.li.org
Departamento de Informatica Fono: +56 32 2654431
Universidad Tecnica Federico Santa Maria +56 32 2654239
Casilla 110-V, Valparaiso, Chile Fax: +56 32 2797513
5875
days inactive
5880
days old
26 comments
15 participants
participants (15)
-
Adam Jackson -
Enrico Scholz -
Hans de Goede -
Horst H. von Brand -
Jason L Tibbitts III -
Jef Spaleta -
Jerry James -
John Reiser -
Josh Boyer -
Lubomir Kundrak -
Mamoru Tasaka -
Patrice Dumas -
Richard W.M. Jones -
Till Maas -
Ville Skyttä