Banned string IsBad function call
There are a number of C/C++ functions that are not considered secure, and are known as 'banned' for that reason. These functions are:
- memory allocation functions like alloca
- string concatenation functions like strcat and strncat
- string copy functions like strcpy and strncpy
- gets functions like gets and _getts
- isbad functions like IsBadWritePtr
- numeric conversion functions like _iota and _itow
- OEM conversion functions like CharToOem
- path functions like _splitpath and makepath
- scan functions like scanf
- string print functions like sprintf and snprintf
- string tokenizing functions like strtok
The SV.BANNED family of checkers finds the use of any of the banned functions in code.
The SV.BANNED.REQUIRED.ISBAD checker flags the use of insecure IsBad functions.
Vulnerability and risk
Most of these prohibited functions were banned because they can lead to buffer overruns (see Understanding buffer overflows).
As well as functions like strcpy and strcat, the banned list includes many of the corresponding 'n' functions, like strncpy and strncat. Although the 'n' functions are often recommended as replacements for their matching non-'n' functions, they are now considered to have issues with non-null termination of overflowed buffers and lack of error returns on overflows.
Mitigation and prevention
Prohibiting the use of these banned APIs is a good way to remove a significant number of code vulnerabilities. The banned functions should be replaced with more secure versions, or the code should be re-designed to avoid the banned function entirely.
To avoid security issues, it is recommended that you use equivalent safe functions for each category of function when the safe equivalents exist. In some cases, there are no replacement functions, so re-architecture of the software is advised. There are no replacement functions for the IsBad functions, because they can mask errors, so you should rewrite the code to avoid using these APIs. Some recommendations are:
- If you need to avoid a crash, wrap your usage of the pointer with __try/__except. However, doing this can easily hide bugs, so you should do this only in areas where it is absolutely critical to avoid a crash (such as crash recovery code) and where you have a reasonable explanation for why the data you're looking at might be invalid. You should also not catch all exceptions, but only types that you know about. Catching all exceptions is just as bad as using IsBad*Ptr.
- For IsBadWritePtr, filling the destination buffer using memset is a preferred way to make sure that output buffers are valid and large enough to hold the amount of space needed.