Re: New Microsoft Security Tool for developers

["Dana Epp" <dana () vulscan com>]

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;
}

This might be more a style issue, but I damn well wanna know if a string I
am passing around is bigger then the allowed size. That means somewhere else
I didn't bounds check properly or I have tainted data. Now noOverflow() can
be checked properly (we should be checking all returns on functions anyways)
against a NULL being passed in, and will let us know if we are overflowing.

Course, knowing Michael there is some deep dark reason for the way he wrote
that to trick us into thinking differently about it. YMMV.

---
Regards,
Dana M. Epp
[Blog: http://silverstr.ufies.org/blog/]


----- Original Message ----- 
From: "Liudvikas Bukys" <[EMAIL PROTECTED]>
To: <[EMAIL PROTECTED]>
Cc: <[EMAIL PROTECTED]>
Sent: Friday, December 12, 2003 7:10 AM
Subject: Re: [SC-L] New Microsoft Security Tool for developers


> The Michael Howard MSDN article on the Windows Application Verifier
> closes with the following "little gem".  I'm afraid that the answer
> does not leap out at me.  Does anyone see through it?
>
> http://msdn.microsoft.com/library/en-us/dncode/html/secure12112003.asp
>
> -----
>
> Okay, now to this little gem. What's wrong with this code? It's a code
sample I saw recently on outlining a safe way to write buffer overrun-free
code.
> void noOverflow(char *str)
> {
> char buffer[10];
> strncpy(buffer,str,(sizeof(buffer)-1));
> buffer[(sizeof(buffer)-1)]=0;
> /* Avoiding buffer flow with the above two lines */
> }