RE: New Microsoft Security Tool for developers

["David Crocker" <dcrocker () eschertech com>]

Ok, here's what I think is wrong with it:

1. It's written in C. C is an *appalling* language for writing anything remotely
critical (including security-critical). Its "array = pointer" mentality
(together with pointer arithmetic, the perils of integer promotion and its
liberal automatic type conversions) must make it a contender for the award for
the worst programming language ever designed. C should be relegated to the sorts
of tasks that assembler was relegated to 20 years ago.

The trouble is, there is a shortage of good programming languages. Java is OK
(just) for many non-real-time applications, but C is pretty much all we have
that is suitable for systems software.

So (turning rant mode off), here's what I think is wrong with it:

2. Whoever wrote it doesn't know what "const" is for. The parameter should be
"const char *str".

3. It should be annotated "/* pre str != null */" or alternatively the function
body should start with "assert(str != null);" to indicate to anyone thinking of
calling it that passing a null parameter is a no-no.

4. Being a pedant, I would replace the 2 instances of "sizeof(buffer)" to
(sizeof(buffer)/sizeof(char))". That way, in 5 years time when someone decides
to support Unicode and replace "char" with "wchar_t", there is at least a chance
that the conversion will be done correctly.

5. As an alternative to (4) (and better), I would define "const int buffersize =
10;" at the beginning, replace "buffer[10]" by "buffer[buffersize]", and replace
both instances of "sizeof(buffer)" by "buffersize". It is very bad practice to
have magic numbers like "10" embedded in the code.

6. There should be a clear comment at the start that the input string will be
truncated without warning if it exceeds 10 characters, thereby changing the
semantics from what the user probably expects. Better in most cases to return an
error code or (if using C++) raise an exception if the input is too long.
Alternatively, allocate the buffer dynamically so it will always be long enough.

David Crocker
Escher Technologies Ltd.
www.eschertech.com


-----Original Message-----
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]
Behalf Of Liudvikas Bukys
Sent: 12 December 2003 15:10
To: [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]
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 */
}