[asterisk-dev] sprintf(buf, "%s ...", buf, ...) and -D_FORTIFY_SOURCE=2

[Tzafrir Cohen]

Below is a message sent to debian-devel by an Ubuntu guy regarding 
one effect of the build flag -D_FORTIFY_SOURCE=2 on some packages.

It seems that Asterisk has only one line of that format:

  channels/misdn_config.c: sprintf(tempbuf, "%s%s, ", tempbuf, iter->msn);

----- Forwarded message from Kees Cook <kees at outflux.net> -----

Date: Sun, 28 Dec 2008 00:42:46 -0800
From: Kees Cook <kees at outflux.net>
To: debian-devel at lists.debian.org

Hi,

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.)

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?

Thanks,

-Kees

[1] https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/305901
[2] http://sourceware.org/bugzilla/show_bug.cgi?id=7075
[3] http://article.gmane.org/gmane.linux.man/639
[4] http://people.ubuntu.com/~kees/sprintf-glibc/logs/

-- 
Kees Cook                                            @debian.org

----- End forwarded message -----

-- 
               Tzafrir Cohen
icq#16849755              jabber:tzafrir.cohen at xorcom.com
+972-50-7952406           mailto:tzafrir.cohen at xorcom.com
http://www.xorcom.com  iax:guest at local.xorcom.com/tzafrir