RE: New Microsoft Security Tool for developers

[Chris Richards <crichards () digitalvoice com>]

> 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 */
> }

I have to admit to a little confusion as to what the purpose of this code
is?  As I see it there are two possible uses:

1) We are using this to sanitize str before it is used elsewhere in the
function.
2) We are using it to sanitize str before it is used by some other function.

The second case just plain won't work because buffer goes out of scope when
we exit the function, leaving str unsanitized, so I'm guessing the first
case.  Therefore, the only problem I can see is that we aren't checking the
validity of str before we start using it.  However, it seems to me like this
code is doing more work than necessary unless we plan on massaging buffer
some way elsewhere.  For example, wouldn't the following achieve the desired
effect of sanitizing str?

if(NULL != str)
        str[9] = '\0';

We've already decreed that str must be at least 10 chars in length by the
assumption that sizeof(buffer) is a valid amount of data to copy from str
using strncpy().  In addition, using the character constant '\0' instead of
the literal number 0 allows us to be character set independent (not that
I've ever seen a character set that didn't use 0 for the null character).

So, what am I missing?

Later,
Chris