Enabling "-Werror=format-security" by default

Florian Weimer fweimer at redhat.com
Thu Nov 21 09:04:28 UTC 2013


On 11/20/2013 06:45 PM, Przemek Klosowski wrote:
> On 11/20/2013 11:13 AM, Jerry James wrote:
>> path_sprintf(), which is static in Game.c. All callers of that
>> function are visible in the same file, and all pass constant strings
>> into the function, which passes those constant strings to sprintf().
>> The function's purpose is to produce a pathname for a file of interest
>> to the caller in the game's installed location. It's too bad that
>> gcc's analysis cannot span function calls inside a compilation unit.
>> There really is nothing wrong with this code.
> Well, the code is inelegant:
>
>   sprintf(path + len, formatted_name);
>
> looks better and avoids the warning if you write it as
>
>   sprintf(&(path[len]), "%s", formatted_name);
>
> which should lead the reader to reflect on whether it makes sense to prevent buffer overflow by
> using %NNs to limit the size of appended name so that it fits within the limits of the path buffer.

You should be using snprintf anyway.  And neither sprintf nor snprintf 
are really suitable for build strings piece-by-piece, unfortunately.

Anyway, adding the "%s" trades a bit of text segment size increase for a 
likely decrease in execution time because the non-format-string argument 
does not have to be parsed for format strings.

-- 
Florian Weimer / Red Hat Product Security Team


More information about the devel mailing list