RE: Spot the Bug – August 14, 2005

I created this bug a couple of weeks ago for a conference I spoke at to illustrate how so few lines of code could be so buggy. Where’s the bug here?

char dest[50], src[100];
int x, y;

if (x=1)
{
strcpy(dest,src);
dest[50] = ‘\0’;
}

return y;

Solution:
Alright, so I admit it — this chunk of code is a bit nonsensical. But I will say that people do make these mistakes all the time, but probably not all at the same time. 🙂

This code has 4 security defects:
1. The if statement with “=” instead of “==”. Many of you would argue that this is of a quality issue than a security issue, and you’d be right. But security is certainly a subset of quality, and this can cause the code to do things that it shouldn’t do.
2. In strcpy, src is larger than dest, causing a buffer overrun.
3. Arrays start at 0, not 1! Therefore, we are writing past the last allocated spot on the array.
4. The variable y is not initialized.

Now that you’ve heard the bad news about all that’s wrong with this code, it’s time for some good news. I bet you didn’t know that Visual Studio 2005 catches all of these problems! Strcpy is caught by the compiler and noted as a warning. We’ve created safe versions of these libraries in Visual Studio 2005 called Safe CRT libraries. PREfast catches the other 3 bugs — even the “=” error. With these tools and proper education, we hope to get developers all over the world wrting more secure code!

 

[Via Spot the Bug!]

The part that I find exciting is that the compiler catches all of these problems. This is excellent. I’m not sure of that status of GCC in this regard, but given GCC development over the past few years, I’ll bet it is either already there, or coming real soon now.