Secure Coding mailing list archives

Re: New Microsoft Security Tool for developers


From: "Jack D. Unrue" <jdunrue () earthlink net>
Date: Sat, 13 Dec 2003 04:15:25 +0000


I agree with the NULL pointer check.

Unless you are paranoid that strncpy() is buggy or hijacked, I
don't believe your second goal is achieved as stated.  But more
importantly, if in fact there is overflow, then the situation is
a whole lot more serious than merits simply returning a flag.  I
would argue that the application should exit (gracefully if possible)
at that point.  At the same time, if strncpy() is not trustable,
then you can't trust the C library and that is a grave design
problem to be solved.

--
Jack Unrue


Dana Epp wrote:

Liudvikas,

On the surface the code doesn't look too bad, and normally we get a false
sense of security when we use the "safer" string functions.

The reality is that what is wrong in that code (well, in my opinion
atleast), is that you can't really tell if there was an overflow happening
or not, and you can pass in a null which could do weird things. There are a
couple of conditions that should be looked for:

1) What happens if str is null?

2) What happens if str is larger than buff? How do we know we have a
problem?

A better way to write it would be something like:

bool noOverflow(char *str)
{
     char buffer[10];

     if( str == NULL )
     {
         /* We should never have a NULL string */
          assert( false );
          return false;
     }

     /* Lets prep our buffer to check for an overflow. Lets nullify the end
char first */
     buffer[sizeof(buffer)-1] = '\0';

    /* Lets copy the string in, max of buffer size */
    strncpy( buffer, str, sizeof(buffer) );

    /* Now lets check if the null at the end of the buffer has been trampled
*/
    if( buffer[sizeof(buffer)-1] != '\0' )
    {
          /* We know of an unsafe string. This has overflowed! */
          return false;
     }

     /* Avoiding buffer flow with the above two lines */
     return true;
}







Current thread: