V645. The function call could lead to the buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold
The analyzer has detected a potential error related to string concatenation. The error might cause buffer overflow. What is unpleasant about these errors is that the program may work stably for a long time as long as the function receives only short strings.
This type of vulnerabilities is characteristic of such functions as 'strncat', 'wcsncat', etc. .
This is the 'strncat' function's description:
char *strncat( char *strDest, const char *strSource, size_t count );
- 'destination' is the recipient string;
- 'source' is the source string;
- 'count' is number of characters to append.
The 'strncat' function is perhaps one of the most dangerous string functions. The danger occurs because its mechanism differs from what programmers expect.
The third argument points at the number of remaining characters that can be placed into it, not the buffer size. Here is a quotation from the function's description in MSDN:
strncat does not check for sufficient space in strDest; it is therefore a potential cause of buffer overruns. Keep in mind that count limits the number of characters appended; it is not a limit on the size of strDest.
Unfortunately, programmers often forget it and use strncat in an inappropriate way. We can distinguish two types of mistakes:
1) Developers think that the 'count' argument is the 'strDest' buffer's size. As a result, proceeding from this misinterpretation they write the following incorrect code:
char newProtoFilter = "...."; strncat(newProtoFilter, szTemp, 2048); strncat(newProtoFilter, "|", 2048);
The programmer believes that he/she is protecting the code against an overflow by passing number 2048 as the third argument. But it's wrong. The programmer is actually telling the code that up to 2048 characters more can be added to the string!
2) People forget that the strncat function will add terminal 0 after copying. Here is an example of dangerous code:
char filename[NNN]; ... strncat(filename, dcc->file_info.filename, sizeof(filename) - strlen(filename));
At first sight you may think the programmer has protected the program from the 'filename' buffer overflow. It's not so. The programmer has subtracted the string length from the array size. It means that if the string is already filled completely, the "sizeof(filename) - strlen(filename)" expression will return one. As a result, one more character will be added to the string, while the terminal null will be written outside the buffer.
Let's clarify this error by a simpler example:
char buf = "ABCD"; strncat(buf, "E", 5 - strlen(buf));
The buffer doesn't have any more space for new characters. It contains 4 characters and the terminal null. The "5 - strlen(buf)" expression equals 1. The strncpy() function will copy the "E" character into the last item of the 'buf' array. The terminal 0 will be written outside the buffer!
To fix the above cited code fragments we need to rewrite them in the following way:
// Sample N1 char newProtoFilter = "...."; strncat(newProtoFilter, szTemp, 2048 - 1 - strlen(newProtoFilter)); strncat(newProtoFilter, "|", 2048 - 1 - strlen(newProtoFilter)); // Sample N2 char filename[NNN]; ... strncat(filename, dcc->file_info.filename, sizeof(filename) - strlen(filename) - 1);
This code cannot be called smart or really safe. It's a much better solution to refuse using functions like 'strncat' in favor of safer ones. For example, use the std::string class or such functions as strncat_s and so on .