Re: mass bug filing for undefined sn?printf use

[José Luis Tallón <jltallon@adv-solutions.net>]

Kees Cook wrote:
> 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.
>   
For LCDproc (as of 5.2), the only matching line is:

    contrib/interface-demo2/if_demo.c:            sprintf(buffer,
"%s,%s", buffer, if_list[i].if_name);

which resulting code is not even installed in binary form.
> Thoughts?
>
> Thanks,
>   
Thank you for taking the time to investigate this issue.