[Pkg-games-ubuntu] [Bug 305901] Re: Intrepid gcc -O2 breaks string appending with sprintf(), due to fortify source patch
Bug Watch Updater
305901 at bugs.launchpad.net
Thu May 26 09:04:55 UTC 2011
Launchpad has imported 9 comments from the remote bug at
http://sourceware.org/bugzilla/show_bug.cgi?id=7075.
If you reply to an imported comment from within Launchpad, your comment
will be sent to the remote bug automatically. Read more about
Launchpad's inter-bugtracker facilities at
https://help.launchpad.net/InterBugTracking.
------------------------------------------------------------------------
On 2008-12-07T17:42:25+00:00 Kees Cook wrote:
Anders Kaseorg noticed that the use of _FORTIFY_SOURCE breaks a specific use of
sprintf (see attached):
$ gcc -O0 -o foo foo.c && ./foo
not fail
$ gcc -O2 -o foo foo.c && ./foo
not fail
$ gcc -O2 -D_FORTIFY_SOURCE=2 -o foo foo.c && ./foo
fail
The original report was filed in Ubuntu, where -D_FORTIFY_SOURCE=2 is enabled by
default: https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/305901
C99 states:
The sprintf function is equivalent to fprintf, except that the output is written
into an array (specified by the argument s) rather than to a stream. A null
character is written at the end of the characters written; it is not counted as
part of the returned value. If copying takes place between objects that overlap,
the behavior is undefined.
The man page does not mention this limitation, and prior to the use of
__sprintf_chk, this style of call worked as expected. As such, a large volume
of source code uses this style of call:
http://web.mit.edu/andersk/Public/sprintf-results
It seems that it would make sense to fix __sprintf_chk, or very loudly mention
the C99-described overlap-is-undefined behavior in sprintf documentation.
Reply at: https://bugs.launchpad.net/glibc/+bug/305901/comments/4
------------------------------------------------------------------------
On 2008-12-07T17:42:53+00:00 Kees Cook wrote:
Created attachment 3095
test case
Reply at: https://bugs.launchpad.net/glibc/+bug/305901/comments/5
------------------------------------------------------------------------
On 2008-12-07T17:49:37+00:00 Andreas Schwab wrote:
sprintf(buf, "%sfoo", buf) is UNDEFINED.
Reply at: https://bugs.launchpad.net/glibc/+bug/305901/comments/6
------------------------------------------------------------------------
On 2008-12-07T18:33:34+00:00 Kees Cook wrote:
Thanks for the clarification. However, I think it is still a bug that the
limitation is not mentioned in the manpage.
Reply at: https://bugs.launchpad.net/glibc/+bug/305901/comments/7
------------------------------------------------------------------------
On 2008-12-07T19:05:38+00:00 Andreas Schwab wrote:
Then contact whoever wrote it.
Reply at: https://bugs.launchpad.net/glibc/+bug/305901/comments/8
------------------------------------------------------------------------
On 2008-12-07T22:56:34+00:00 Jakub Jelinek wrote:
man 3p sprintf certainly documents it:
"If copying takes place between objects that overlap as a result of a call
to sprintf() or snprintf(), the results are undefined."
Reply at: https://bugs.launchpad.net/glibc/+bug/305901/comments/18
------------------------------------------------------------------------
On 2008-12-07T23:38:40+00:00 Petr Baudis wrote:
I have submitted a patch for linux-manpages:
http://thread.gmane.org/gmane.linux.man/639
Reply at: https://bugs.launchpad.net/glibc/+bug/305901/comments/19
------------------------------------------------------------------------
On 2008-12-19T16:57:40+00:00 Michael Kerrisk wrote:
(In reply to comment #6)
> I have submitted a patch for linux-manpages:
> http://thread.gmane.org/gmane.linux.man/639
I've applied the following patch for man-pages-3.16.
--- a/man3/printf.3
+++ b/man3/printf.3
@@ -133,6 +133,17 @@ string that specifies how subsequent arguments (or
arguments accessed via
the variable-length argument facilities of
.BR stdarg (3))
are converted for output.
+
+C99 and POSIX.1-2001 specify that the results are undefined if a call to
+.BR sprintf (),
+.BR snprintf (),
+.BR vsprintf (),
+or
+.BR vsnprintf ()
+would cause to copying to take place between objects that overlap
+(e.g., if the target string array and one of the supplied input arguments
+refer to the same buffer).
+See NOTES.
.SS "Return value"
Upon successful return, these functions return the number of characters
printed (not including the
@@ -851,6 +862,26 @@ and conversion characters \fBa\fP and \fBA\fP.
glibc 2.2 adds the conversion character \fBF\fP with C99 semantics,
and the flag character \fBI\fP.
.SH NOTES
+Some programs imprudently rely on code such as the following
+
+ sprintf(buf, "%s some further text", buf);
+
+to append text to
+.IR buf .
+However, the standards explicitly note that the results are undefined
+if source and destination buffers overlap when calling
+.BR sprintf (),
+.BR snprintf (),
+.BR vsprintf (),
+and
+.BR vsnprintf ().
+.\" http://sourceware.org/bugzilla/show_bug.cgi?id=7075
+Depending on the version of
+.BR gcc (1)
+used, and the compiler options employed, calls such as the above will
+.B not
+produce the expected results.
+
The glibc implementation of the functions
.BR snprintf ()
and
Reply at: https://bugs.launchpad.net/glibc/+bug/305901/comments/20
------------------------------------------------------------------------
On 2008-12-24T17:40:22+00:00 Kees Cook wrote:
Created attachment 3625
work-around pre-trunc behavior
This patch restores the prior sprintf behavior. Looking through
_IO_str_init_static_internal seems to indicate that nothing actually requires
"s" to lead with a NULL. Is there anything wrong with this work-around, which
could be used until the number of affected upstream sources is not quite so
large?
Reply at: https://bugs.launchpad.net/glibc/+bug/305901/comments/27
** Changed in: glibc
Importance: Unknown => Medium
--
You received this bug notification because you are a member of
Debian/Ubuntu Games Team, which is subscribed to billard-gl in Ubuntu.
https://bugs.launchpad.net/bugs/305901
Title:
Intrepid gcc -O2 breaks string appending with sprintf(), due to
fortify source patch
Status in The GNU C Library:
Invalid
Status in “4g8” package in Ubuntu:
Invalid
Status in “abiword” package in Ubuntu:
Invalid
Status in “asterisk” package in Ubuntu:
Invalid
Status in “atomicparsley” package in Ubuntu:
Invalid
Status in “audacious-plugins” package in Ubuntu:
Invalid
Status in “barnowl” package in Ubuntu:
Invalid
Status in “billard-gl” package in Ubuntu:
Invalid
Status in “binutils” package in Ubuntu:
Invalid
Status in “blender” package in Ubuntu:
Invalid
Status in “ctn” package in Ubuntu:
Invalid
Status in “gcc-4.3” package in Ubuntu:
Invalid
Status in “glibc” package in Ubuntu:
Fix Released
Status in “hypermail” package in Ubuntu:
Invalid
Status in “mpeg4ip” package in Ubuntu:
Invalid
Status in “nagios-plugins” package in Ubuntu:
Invalid
Status in “owl” package in Ubuntu:
Invalid
Status in “xmcd” package in Ubuntu:
Invalid
Status in “4g8” source package in Intrepid:
Invalid
Status in “abiword” source package in Intrepid:
Invalid
Status in “asterisk” source package in Intrepid:
Invalid
Status in “atomicparsley” source package in Intrepid:
Invalid
Status in “audacious-plugins” source package in Intrepid:
Invalid
Status in “barnowl” source package in Intrepid:
Invalid
Status in “billard-gl” source package in Intrepid:
Invalid
Status in “binutils” source package in Intrepid:
Invalid
Status in “blender” source package in Intrepid:
Invalid
Status in “ctn” source package in Intrepid:
Invalid
Status in “gcc-4.3” source package in Intrepid:
Invalid
Status in “glibc” source package in Intrepid:
Fix Released
Status in “hypermail” source package in Intrepid:
Invalid
Status in “mpeg4ip” source package in Intrepid:
Invalid
Status in “nagios-plugins” source package in Intrepid:
Invalid
Status in “owl” source package in Intrepid:
Invalid
Status in “xmcd” source package in Intrepid:
Invalid
Status in “4g8” source package in Jaunty:
Invalid
Status in “abiword” source package in Jaunty:
Invalid
Status in “asterisk” source package in Jaunty:
Invalid
Status in “atomicparsley” source package in Jaunty:
Invalid
Status in “audacious-plugins” source package in Jaunty:
Invalid
Status in “barnowl” source package in Jaunty:
Invalid
Status in “billard-gl” source package in Jaunty:
Invalid
Status in “binutils” source package in Jaunty:
Invalid
Status in “blender” source package in Jaunty:
Invalid
Status in “ctn” source package in Jaunty:
Invalid
Status in “gcc-4.3” source package in Jaunty:
Invalid
Status in “glibc” source package in Jaunty:
Fix Released
Status in “hypermail” source package in Jaunty:
Invalid
Status in “mpeg4ip” source package in Jaunty:
Invalid
Status in “nagios-plugins” source package in Jaunty:
Invalid
Status in “owl” source package in Jaunty:
Invalid
Status in “xmcd” source package in Jaunty:
Invalid
Bug description:
Binary package hint: gcc-4.3
In Hardy and previous releases, one could use statements such as
sprintf(buf, "%s %s%d", buf, foo, bar);
to append formatted text to a buffer buf. Intrepid’s gcc-4.3, which has fortify source turned on by default when compiling with -O2, breaks this pattern. This introduced mysterious bugs into an application I was compiling (the BarnOwl IM client).
Test case: gcc -O2 sprintf-test.c -o sprintf-test
<http://web.mit.edu/andersk/Public/sprintf-test.c>:
#include <stdio.h>
char buf[80] = "not ";
int main()
{
sprintf(buf, "%sfail", buf);
puts(buf);
return 0;
}
This outputs "not fail" in Hardy, and "fail" in Intrepid.
The assembly output shows that the bug has been introduced by
replacing the sprintf(buf, "%sfail", buf) call with __sprintf_chk(buf,
1, 80, "%sfail", buf). A workaround is to disable fortify source (gcc
-U_FORTIFY_SOURCE).
One might argue that this usage of sprintf() is questionable. I had
been under the impression that it is valid, and found many web pages
that agree with me, though I was not able to find an authoritative
statement either way citing the C specification. I decided to
investigate how common this pattern is in real source code.
You can search a source file for instances of it with this regex:
pcregrep -M 'sprintf\s*\(\s*([^,]*)\s*,\s*"%s[^"]*"\s*,\s*\1\s*,'
To determine how common the pattern is, I wrote a script to track down instances using Google Code Search, and found 2888 matches:
<http://web.mit.edu/andersk/Public/sprintf-results>
(For the curious: the script uses a variant of the regex above. I had to use a binary search to emulate backreferences, which aren’t supported by Code Search, so the script makes 46188 queries and takes a rather long time to run. The source is available at <http://web.mit.edu/andersk/Public/sprintf-codesearch.py>.)
My conclusion is that, whether or not this pattern is technically
allowed by the C specification, it is common enough that the compiler
should be fixed, if that is at all possible.
More information about the Pkg-games-ubuntu
mailing list