(In reply to Kees Cook from comment #14) > So I'd like to bring this back up and reiterate the issue: there is no > benefit to the early truncation, and it actively breaks lots of existing > software (which is why Debian and Ubuntu have had this fix for 10 years now). > > What is the _benefit_ of early truncation that justifies breaking so many > existing cases?
ideally sprintf, snprintf and sprintf_chk would be able to share code and have identical behaviour (currently there is a lot of duplicated logic in glibc with a potential for inconsistent behaviour). that said, i think _FORTIFY_SOURCE should detect undefined behaviour if possible since it's a bug that breaks portability. note that it does not matter what guarantees a library documents: there are plenty of precedents for compiler optimizations to break code based on ub in library calls, a compiler can remove all code paths leading to a sprintf(s, "%s", s), trying to make such code work in glibc is just hiding a ticking time bomb. -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to binutils in Ubuntu. https://bugs.launchpad.net/bugs/305901 Title: Intrepid gcc -O2 breaks string appending with sprintf(), due to fortify source patch Status in GLibC: Confirmed 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. To manage notifications about this bug go to: https://bugs.launchpad.net/glibc/+bug/305901/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : touch-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp
