[Xastir-dev] Proposal to clean up some warnings

[Tom Russo]

On Wed, Jun 09, 2010 at 12:11:35PM -0600, we recorded a bogon-computron collision of the <russo at bogodyn.org> flavor, containing:
> On Wed, Jun 09, 2010 at 10:42:52AM -0700, we recorded a bogon-computron collision of the <curt.we7u at gmail.com> flavor, containing:
> > On Wed, 9 Jun 2010, Jerry Dunmire wrote:
> > 
> > > While working on the OSM patch I noticed that the compile produces a
> > > lot of warnings and I would like to try and reduce the number a bit.
> > >
> > > The first one I would like to tackle is:
> > >   warning: format not a string literal and no format arguments
> > 
> > > Does anyone have a problem with this change?
> > 
> > Yea, me.  I spent a lot of time adding all the xastir_snprintf()
> > stuff a while back to make Xastir much more bulletproof.  There are
> > hundreds of those calls that were changed at the time.
> > 
> > I'm not seeing _any_ compiler warnings such as you describe when I
> > do a "make clean;make".  
> 
> [...]
> 
> > I suspect what you're seeing are dependent on your particular
> > compiler and it's default flags, along with the CFLAGS that Xastir
> > uses.  In my case I'm compiling on OpenSuSE-11.1 or 11.2, 64-bit
> > x86.  I have a couple of machines still that run 32-bit, similar OS.
> 
> 
> This is a warning produced by newer GCC versions.  I am seeing it on newer
> Ubuntu VMs with GCC 4.4.  I believe the intent is to guard against the 
> possibility that the string variable might contain formatting characters
> that would produce a run time error, e.g. the effect would be similar to 
> the obvious mistake:
>    snprintf(dest,length,"This %s is an error\n");

A little googling turns up another reason for this warning to have crept into
GCC.  Apparently it is a potential security issue if printf and friends
might be using a string variable that came from untrusted input.  The 
untrusted input could sneak in formatting characters and cause havoc.

>From the GCC 4.4 manual:
-----------
-Wformat-security
    If -Wformat is specified, also warn about uses of format functions that represent possible security problems. At present, this warns about calls to printf and scanf functions where the format string is not a string literal and there are no format arguments, as in printf (foo);. This may be a security hole if the format string came from untrusted input and contains `%n'. (This is currently a subset of what -Wformat-nonliteral warns about, but in future warnings may be added to -Wformat-security that are not included in -Wformat-nonliteral.) 
-------

Apparently -Wformat-security is now a default.

Fixing the issue by adding format strings to the snprintf calls seems the sane
approach, as it will not break older builds, will retain Curt's desired
bulletproofing, and will shut GCC up by fixing the perceived security issue
(which in this case is probably not a security issue).

-- 
Tom Russo    KM5VY   SAR502   DM64ux          http://www.swcp.com/~russo/
Tijeras, NM  QRPL#1592 K2#398  SOC#236        http://kevan.org/brain.cgi?DDTNM
 "It is better to live on your feet than to die with your knees."
  -- Mil Millington on running, in Instructions for Living Someone Else's Life