On Sun, 28 Dec 2008 00:42:46 -0800 Kees Cook <k...@outflux.net> wrote:
> I'd like to seek advice before I perform a mass-bug filing for this > unstable (though semi-common) use of "sprintf" and "snprintf": > > sprintf(buf, "%s foo %d %d", buf, var1, var2); > > This is used in many upstreams to perform a format-string-handling > version of strcat. > > This was originally noticed by Anders Kaseorg in Ubuntu[1], since > -D_FORTIFY_SOURCE=2 triggers a change in behavior (buf is truncated before > handling the rest of the format string instead of performing the concat). > > Upstream glibc points out[2] that using sprintf in this way is undefined > under C99, and the man pages have now been updated[3] to reflect this. > (Though I believe it is possible to patch glibc to avoid the change in > behavior, it's probably best to work on fixing all the upstreams.) > > In Debian, some tools already compile natively with -D_FORTIFY_SOURCE=2, > and some have Build-Depends on "hardening-wrapper", which enables this > compiler flag. As such, it seems sensible to have all affected packages > fixed since the results of such a call could change. (Though it is not an > RC issue.) By all affected packages, do you mean packages that use the code or packages that use the code *AND* compile with or Build-Depend on hardening-wrapper? IMHO any bugs filed merely due to the presence of the code without the means to trigger the error in normal builds should be wishlist. Re: Debian GPE team <pkg-gpe-maintain...@lists.alioth.debian.org> gpe-conf (U) gpe-conf, being Gtk+ and therefore GLib can simply switch to using g_strconcat or g_sn?printf or g_strdup_printf and avoid all these problems. In the specific case of gpe-conf, only two of the files using this code do not already include gtk/gtk.h or glib/glib.h so it is only sensible to use the GLib functions instead for most if not all occurrences. (Indeed, in many cases, the use of a newly allocated string to be freed later, instead of a static fixed buffer has other benefits elsewhere.) > And, a possible solution from Anders Kaseorg... > This example sprintf() call could be fixed as follows: > -sprintf(buf, "%s plus %d", buf, k); > +sprintf(buf + strlen(buf), " plus %d", k); > Similarly, an invalid snprintf() call could be fixed as follows: > -snprintf(buf, buflen, "%s plus %d", buf, k); > +snprintf(buf + strlen(buf), buflen - strlen(buf), " plus %d", k); > > Attached is a list of affected packages, generated via: > > pcregrep -M 'sprintf\s*\(\s*([^,]*)\s*,\s*"%s[^"]*"\s*,\s*\1\s*,' > pcregrep -M 'snprintf\s*\(\s*([^,]*)\s*,[^,]*,\s*"%s[^"]*"\s*,\s*\1\s*,' > > The logs for individual packages can be seen here[4]. I've tried to trim > out stuff that was Ubuntu-specific or not relevant, so apologies in advance > if there are incorrect (or missing) things in the list. > > Thoughts? Split the list according to packages that merely match the regexp and those that match the regexp *AND* match a second regexp indicating that the build system either uses -D_FORTIFY_SOURCE=2 or hardening-wrapper? -- Neil Williams ============= http://www.data-freedom.org/ http://www.nosoftwarepatents.com/ http://www.linux.codehelp.co.uk/
pgpC1REHsLAUm.pgp
Description: PGP signature
