On 07/12/2016 04:01 AM, Florian Weimer wrote:
On 07/12/2016 11:54 AM, Jakub Jelinek wrote:
On Tue, Jul 12, 2016 at 11:51:50AM +0200, Florian Weimer wrote:
On 07/01/2016 08:15 PM, Martin Sebor wrote:
The attached patch enhances compile-time checking for buffer overflow
and output truncation in non-trivial calls to the sprintf family of
functions under a new option -Wformat-length=[12]. This initial
patch handles printf directives with string, integer, and simple
floating arguments but eventually I'd like to extend it all other
functions and directives for which it makes sense.
I tried your patch with the following code, which is close to a
real-world
example:
#include <stdio.h>
void print (const char *);
void
format_1 (unsigned address)
{
unsigned char a = address >> 24;
unsigned char b = address >> 16;
unsigned char c = address >> 8;
unsigned char d = address;
char buf[15];
sprintf ("%u.%u.%u.%u", buf, a, b, c, d);
Are you sure this is real-world code? sprintf's first argument is the
buffer and second the format string, so if this doesn't warn at compile
time, it will surely crash at runtime when trying to store into .rodata.
Argh! You are right, I swapped the arguments.
And further attempts showed that I was missing -D_FORTIFY_SOURCE=2. With
it, I get a nice diagnostic. Wow!
Thanks for giving it a try! Based on the feedback I received
I've since updated the patch and will post the latest version
for review soon. In simple cases like this one it warns even
without _FORTIFY_SOURCE or optimization (though without the
latter it doesn't benefit from VRP information). Let me see
about adding a warning to detect and warn when the arguments
are transposed.
$ /build/gcc-49905/gcc/xgcc -B /build/gcc-49905/gcc -S -Wall -Wextra
-Wpedantic -Wformat-length=2 xyz.c
xyz.c: In function ‘format_1’:
xyz.c:13:29: warning: may write a terminating nul past the end of the
destination [-Wformat-length=]
sprintf (buf, "%u.%u.%u.%u", a, b, c, d);
^
xyz.c:13:3: note: destination size is ‘15’ bytes, output size between
‘8’ and ‘16’
sprintf (buf, "%u.%u.%u.%u", a, b, c, d);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
xyz.c: In function ‘format_2’:
xyz.c:21:3: warning: ‘%u’ directive writing between ‘1’ and ‘10’ bytes
into a region of size ‘4’ [-Wformat-length=]
sprintf (buf, "%u.%u.%u.%u",
^
xyz.c:21:3: note: using the range [‘1u’, ‘2147483648u’] for directive
argument
xyz.c:21:3: note: destination size is ‘15’ bytes, output size between
‘4’ and ‘22’
sprintf (buf, "%u.%u.%u.%u",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
address >> 24,
~~~~~~~~~~~~~~
(address >> 16) & 0xff,
~~~~~~~~~~~~~~~~~~~~~~~
(address >> 8) & 0xff,
~~~~~~~~~~~~~~~~~~~~~~
address & 0xff);
~~~~~~~~~~~~~~~
xyz.c:21:3: note: destination size is ‘15’ bytes, output size between
‘6’ and ‘33’
Martin
