[cfe-commits] [PATCH] review request: strcpy() security checker CWE-119

[Lenny Maiorani]

On 03/31/2011 02:05 PM, Marshall Clow wrote:
> On Mar 31, 2011, at 12:34 PM, Lenny Maiorani wrote:
>
>> While I am at it, poking around in the string functions, here is a security syntax checker for strcpy() which causes the Static Analyzer to generate a warning any time the strcpy() function is used with a note suggesting to use a function which provides bounded buffers.
>>
>> I included in the warning description the CWE number. Is this useful?
>>
>> Should the warning description also contain suggestions like strncpy() and strlcpy()? Since there are a number of options I left that suggestion out in leiu of the CWE number.
> Can you check and see if the source is a literal constant, and the size of the destination is big enough, and in that case not issue the warning?
>
> 	char buffer[10];
> 	strcpy ( buffer, "Hi Mom!" );
>
> should not trigger a warning.
>
> -- Marshall
>
> P.S.	I think it's great that you're adding these!
>
Marshall,

There are two types of warnings (maybe more) generated around strcpy(). 
This one is the security warning. The intention here is that strcpy() 
shouldn't be used and strncpy() or strlcpy() should be used instead. The 
other instance is validating the buffer sizes. This is done in the 
CStringChecker. You can run both of these checkers using something like:

clang -cc1 -analyze 
-analyzer-checker=core,cplusplus.experimental.CString,deadcode.experimental.UnreachableCode,security.experimental.SecuritySyntactic 
-analyzer-store=region

With your example above, only the security warning would be generated. 
However, if the buffer was 3 bytes long then both the security and the 
buffer overrun warnings would be generated.

Unfortunately for many people passively using CSA, they don't know that 
many checkers are disabled by default. They need more run-time before 
high enough confidence and low enough false positive rates are proven. 
This is one of them. I hope this clarifies.

-Lenny