Feature or enhancement

Proposal:

The program performs a buffer copy or write operation with no upper limit on the size of the copy. By analyzing the bounds of the expressions involved, it appears that certain inputs will cause a buffer overflow to occur in this case. In addition to causing program instability, techniques exist which may allow an attacker to use this vulnerability to execute arbitrary code.

Recommendation

Always control the length of buffer copy and buffer write operations.strncpy should be used overstrcpy,snprintf over sprintf, and in other cases 'n-variant' functions should be preferred.

intsayHello(uint32_tuserId){charbuffer[17];if (userId>9999)returnUSER_ID_OUT_OF_BOUNDS;// BAD: this message overflows the buffer if userId >= 1000,// as no space for the null terminator was accounted forsprintf(buffer,"Hello, user %d!",userId);MessageBox(hWnd,buffer,"New Message",MB_OK);returnSUCCESS;}

the call tosprintf writes a message of 14 characters (including the terminating null) plus the length of the string conversion ofuserId into a buffer with space for just 17 characters. WhileuserId is checked to occupy no more than 4 characters when converted, there is no space in the buffer for the terminating null character ifuserId >= 1000. In this case, the null character overflows the buffer resulting in undefined behavior.

References

CERT C Coding Standard:STR31-C. Guarantee that storage for strings has sufficient space for character data and the null terminator
CERT C++ Coding Standard:STR50-CPP. Guarantee that storage for strings has sufficient space for character data and the null terminator

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response