"Secure coding" audit checkers and Kerberos

[Ken Raeburn]

On Oct 14, 2008, at 21:47, Luke Howard wrote:
>> * Instead of strcpy or strcat, use memcpy.  Remember to ensure that
>> the string is terminated if you are not copying a terminator.
>
> What about using strlcpy/strlcat (providing implementations for
> platforms that don't support them).

And then check for truncation, and add error paths or abort calls?  As  
Greg said, silent truncation can lead to security problems just as  
buffer overruns can.  If we skip the truncation tests on the grounds  
that we've verified the code is consistent and correct in its use of  
lengths, then either we can just use memcpy, or we're not *that*  
confident of our checks and should keep the truncation tests.  If we  
put in the tests, now we need error recovery for more than just failed  
allocation, or we have "assert" or "abort" calls littered through the  
code, which some coding standards would regard as rather ugly.

Personally, my preference is to use small building blocks that can be  
verified to be correct, and build on them.  The "strdup" function is  
an obvious example, which we already use in a lot of places;  
"asprintf"/"vasprintf" are hairier (to implement and verify) but also  
valid examples.  I haven't looked at many of the examples, but there  
are probably other simple things that could be implemented as helper  
functions, like "copy out krb5_data contents into a null-terminated C  
string, but (... do something) if there's a null byte in the content".

The GSS buffer handling is another issue; we've had a bunch of cases  
in the past where code used "%s" to print the buffer contents,  
ignoring the fact that the GSS-API C bindings don't guarantee null  
termination of strings stored in buffer handles, but "%.*s" can supply  
the length (when appropriately cast, after range checking) and pointer  
data together so as to avoid overrunning the buffer.  (Should check on  
this: I'm not certain *printf are required not to read past N bytes,  
though they are required not to output more than that.)

Ken