On 04/29/2017 11:50 AM, Sebastian Noack wrote:
Hi everybody,
If you use -D_FORTIFY_SOURCE=2 (or 1), some additionally checks, both,
at compile- and run-time, supposed to catch buffer overflows, are
performed. Debian (and some other distributions) use this option by
default, when packaging software. And since I generally build my code
with -Werror, I cannot just ignore the warnings triggered by
_FORTIFY_SOURCE, when packaging my software.
First I thought the addional compiler warnings are a good thing, and
it might be a good practice to just change my code. But after looking
into them, it turned out that most (if not all) of the time, the
unused-result warnings triggered by _FORTIFY_SOURCE, are IMO
ridicolous, suggest less maintainable code, and are unrelated of
buffer overflows. One example:
int value = -1;
fscanf(fh, "%u", &value);
This is causing a warning, apparently you should handle the return
value of fscanf, for no good reason whatsoever. The output variable is
initialized with a value, indicating an invalid value. The behavior is
completely the same compared to the more verbose alternative,
_FORTIFY_SOURCE is suggesting:
int value;
if (fscanf(fh, "%u", &value) != 1)
value = -1;
With _FORTIFY_SOURCE you get a warning every time you try to read or
write some data and don't explicitly handle the return value of the
respective stdlib function.
The warning is triggered by the fscanf function being decorated
with attribute warn_unused_result in the <stdio.h> header. The
header is part of libc, not GCC, so the appropriate place to
raise a concern with it would on the libc mailing list (such as
libc-help or libc-alpha for Glibc).
That being said, the reason for the warning is to help detect
unsafe assumptions about the function succeeding, such as in
both of the code snippets above. The code doesn't make it
possible to distinguish between fscanf failing or successfully
reading a -1.
It's possible that the distinction doesn't make a difference
downstream from the function call, but it very well could, and
GCC doesn't have a way to tell which it is. The warning serves
to alert the programmer to the risks in case it does.
But I generally try to keep my code as
simple as possible (which also reduces potential bugs), and most of
the time all I could do if an IO function fails, would be logging an
error message. But I'd rather avoid to bloat my code with unnecessary
error handling, that is only triggered in rather theoretical
scenarios, except when it has security implications.
If you find the -Wunused-result warnings unhelpful you should be
able to disable them by -Wno-unused-result, either on the command
line or by adding pragma GCC diagnostic ignored "-Wno-unused-result"
above the call. The warnings can also be suppressed simply by
testing the return value and doing nothing with it (e.g., by
'if (fscanf(fh, "%u", &value)) { /* do nothing */ }').
But what could
possibly go wrong, if I keep writing to stdout after it has been
closed by the parent process, for example?
The answer obviously depends on what relies on the integrity
of the output, but I'm not sure I see how the question is
relevant to the examples above (it's fscanf that's decorated
with warn_unused_result, not fprintf). There is no warning
for printf/fprintf calls whose return value is unused,
precisely because it is commonly ignored.
It should go without saying that unlike writing output to
a stream, reading input from one and ignoring errors is much
more likely to indicate a flaw.
Or why should I have to
check whether all bytes have been read into an (initialized) buffer,
before passing it to a parser which must be able to deal with invalid
input, anyway. Regardless of the aspect that addressing these would
significantly bloat my code, I also feel that it would make my code
less robust, by adding error handling code that won't (or can't even
easily) be triggered during testing. And what does all of this has to
do with buffer overflows, anyway?
These again would be questions best asked on libc-help, but the
answer is apparent from the definition of the Glibc __wur macro
the fscanf function in <stdio.h> is decorated with:
/* If fortification mode, we warn about unused results of certain
function calls which can lead to problems. */
#if __GNUC_PREREQ (3,4)
# define __attribute_warn_unused_result__ \
__attribute__ ((__warn_unused_result__))
# if __USE_FORTIFY_LEVEL > 0
# define __wur __attribute_warn_unused_result__
# endif
Martin
PS You can find past discussions on this subject in various
online forums such as Stack Overflow:
http://stackoverflow.com/search?q=ignoring+return+value+of+fscanf
