[Bug c/49905] New: Better sanity checking on sprintf src & dest to produce warning for dodgy code ?

[Bug c/49905] New: Better sanity checking on sprintf src & dest to produce warning for dodgy code ?

[dcb314 at hotmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49905

           Summary: Better sanity checking on sprintf src & dest to
                    produce warning for dodgy code ?
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: dcb314@hotmail.com


I just tried to compile the following C++ code with the latest trunk
snapshot 20110723 on an AMD x86_64 box.

# include <stdio.h>

void f()
{
    char * p = new char [4];
    char q[4];

    sprintf(p, "ian");  // Legal
    sprintf(q, "ian");

    sprintf(p, "bert"); // One over
    sprintf(q, "bert");

    sprintf(p, "harry");    // definately wrong.
    sprintf(q, "harry");

    sprintf(p, "%s", "harry");  // more subtle.
    sprintf(q, "%s", "harry");

    sprintf(p, "%s %s", "ab", "cd");    // more subtle still.
    sprintf(q, "%s %s", "ab", "cd");

    sprintf(p, "%s %d", "ab", 1000);    // overpoints.
    sprintf(q, "%s %d", "ab", 1000);
}

Much to my surprise, the compiler, even with lots of flags, said not much

bash-4.2$ ~/gcc/20110723/results/bin/g++ -c -O2 -Wall -Wextra -pedantic
bug29.cc
bash-4.2$

I'd have expected a few warnings, at very least.

Surely something in the compiler could be done to check that sprintf is
called, the destination buffer size is known, the minimum source buffer
size is known, and compare the two to make sure the source fits inside
the destination ?

Such a warning will in my experience find plenty of bugs in application code.

[Bug middle-end/49905] Better sanity checking on sprintf src & dest to produce warning for dodgy code ?

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49905

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-07-29 21:10:06 UTC ---
Just use also -D_FORTIFY_SOURCE=1 -O2 or -D_FORTIFY_SOURCE=2 -O2.
For the first three overflows on q you'll get compile time warnings, and for
all overflows on q you'll get the program killed at runtime.
If you use char * p = (char *) malloc (4);
instead of char * p = new char [4];
you'll get protection for the p overflows too, I'll see if
__builtin_object_size could be taught about C++ new, at least some forms
thereof.

[Bug middle-end/49905] Better sanity checking on sprintf src & dest to produce warning for dodgy code ?

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49905

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-08-04 07:40:29 UTC ---
Author: jakub
Date: Thu Aug  4 07:40:24 2011
New Revision: 177316

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=177316
Log:
    PR middle-end/49905
    * tree.h (init_attributes): New prototype.
    * attribs.c (init_attributes): No longer static.

    * decl.c (cxx_init_decl_processing): Add alloc_size (1) attribute
    for operator new and operator new [].  Call init_attributes.

    * g++.dg/ext/builtin-object-size3.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/ext/builtin-object-size3.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/attribs.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree.h