What C/C++ functions are most often used incorrectly and can lead to buffer overflows?

Question

I've been asked to maintain a large C++ codebase full of memory leaks. While poking around, I found out that we have a lot of buffer overflows that lead to the leaks (how it got this bad, I don't ever want to know).

I've decided to removing the buffer overflows first, starting with the dangerous functions. What C/C++ functions that are most often used incorrectly and can lead to buffer overflow?

For compiler and/or tools used to help look for buffer overrun, I've created another question that deals with this

Answer 1

Here's some functions that I found that are dangerous:

• gets() - It doesn't check the length of the variable and can overwrite memory if the input is bigger than the variable's buffer.
• scanf() - I'm so glad that Visual Studio told me this function is deprecated. This was an easy fix.
• strcpy() - If the source's memory space is bigger than the destination's, the data after the destination is overwritten.

Answer 2

The following link should give you a comprehensive look at security functions in C++ (ones that are post-fixed with '_s' to fix problems like overflows): http://msdn.microsoft.com/en-us/library/8ef0s5kh(VS.80).aspx

EDIT: This link contains the specific functions that have been replaced: http://msdn.microsoft.com/en-us/library/wd3wzwts(VS.80).aspx

EDIT: I should mention these are Microsoft methods, but the link is still useful for identifying functions that were deemed a red flag.

Answer 3

Unfortunately any array can result in a buffer overflow:

uint32_t foo[3];
foo[3] = WALKED_OFF_END_OF_ARRAY;

In terms of functions, sprintf will happily walk off the end of the buffer. It can be replaced by snprintf.

Answer 4

In general, any function that does not check bounds in the arguments. A list would be

• gets()
• scanf()
• strcpy()
• strcat()

You should use size limited versions like stncpy, strncat, fgets, etc. Then be careful while giving the size limit; take into consideration the '\0' terminating the string.

Also, arrays are NOT bound checked in C or C++. The following example would cause errors. See off by one error

int foo[3];
foo[3] = WALKED_OFF_END_OF_ARRAY;

edit: Copied answers of @MrValdez , @Denton Gentry

Answer 5

Basically, anything which accept a pointer and writes to it, without checking the length. So thing like strcpy(), sprintf() etc.

Answer 6

Memcpy() is another dangerous one.

Any loop accessing an array is a danger point, because there's no stopping going beyond the end of array.

Memory Leaks are caused by allocating memory, and not freeing it. Constructor and destructors should be another strong review point, the latter to make sure any allocated memory is freeded.

Answer 7

Which version of visual studio are you using? In 2008 with all warnings enabled, all the functions you mention (and more) warn you that they are deprecated.

Perhaps you could check that all warnings are turned on and let the compiler do the hard work for you?

As a side note, the excellent writing secure code does a great job explaining the different the pitfalls of some of the older functions.

Answer 8

I have somewhat the same problem on the code base I work on. My advice: be wary of any C functions that look like str*() and mem*(). Also be wary of anything that takes a pointer to a buffer, without a length. Since it seems like you have the chance to use C++ I would in the most egregious cases try to use C++ containers for things: vector, string, map, etc. These make your life a lot easier.

Also, automated problem detection tools are wonderful to have. If you can use valgrind I would recommend it. Also Rational Purify is extremely powerful, though not cheap.

Answer 9

The question is starting at the wrong end, I'm afraid. It's presuming that buffer overruns happen in other functions. The most common cause is operator++, in my experience, or alternatively a lack of operator!=.

The best solution to find a batch of those is /GS in Visual Studio 2005/8. It won't find all of them, but it's a cheap way to reduce the amount of manual work needed.

Answer 10

Valgrind is your new best friend.

valgrind --tool=memcheck --leak-check=full ./a.out

Answer 11

An additional gotcha in C is the "strncpy()" function. Many people do not realize that it is free to return a string that is not null terminated.