"Secure coding" audit checkers and Kerberos
ghudson at MIT.EDU
Tue Oct 14 21:06:26 EDT 2008
We've been asked to stop using "traditionally insecure" functions
(like strcpy) in order to make krb5 code conform to the standards of
code bases which incorporate it.
I'm developing coding standards in line with this goal. For the
moment, I'm working from Coverity's "secure coding" checker, which is
flagging the use of the following functions in the code base:
If there are other functions flagged by Solaris lint or similar tools,
please let me know.
I would prefer not to recommend truncating functions as alternatives
in most cases. Silent truncation can sometimes result in bugs or
security holes which cannot be detected by static analysis tools.
Here are my initial ideas, as a starting point for discussion:
* Instead of strcpy or strcat, use memcpy. Remember to ensure that
the string is terminated if you are not copying a terminator.
* Instead of using sprintf to convert an integral type into a string,
use snprintf with an 80-byte buffer.
* Instead of using sprintf to compose a human-readable string (such as
an error message), use snprintf or asprintf.
* Instead of using sprintf to compose a pathname or other internal
string, use asprintf or manual composition. asprintf will require a
check for allocation failure.
* Instead of using sprintf to compose a human-readable message, use
* Instead of using sscanf or fscanf, use manual parsing; use strtol or
similar to parse numbers.
Rationale and caveats:
* 80 bytes is enough space for a 256-bit integral type (including sign
byte and null terminator). It will be a while before processors get
past that point. Using buffers large enough for even larger
integral types could start running into stack space issues on mobile
devices or in threaded environments. We could recommend asprintf,
but that means checking for a memory allocation failure for every
single integer-to-string conversion, which is pretty burdensome.
* We already have a mechanism to ensure that snprintf and asprintf
exist. We do not currently have a mechanism to ensure that strlcpy
and strlcat exist, and we don't use those in our code base
currently. We could add them if we decide we want to allow their
use, of course.
* strncpy's behavior is too bad to recommend even in cases where
truncation is acceptable.
* Using memcpy to copy a string constant into a buffer is a pain
unless the string constant is short enough that its length is
obvious. One option is to define a macro for this case; I'd like to
hear discussion before proposing something concrete.
* Manual parsing in place of sscanf can be pretty laborious in places,
but there aren't a lot of good alternatives. There are only 11 uses
of sscanf or fscanf that I counted, so it's not that big of a deal.
* I haven't addressed the rand-type functions above. Obviously, using
krb_c_random_* is ideal and we already do that in most places. We
don't use random/lrand48/rand often and it's generally in a test
functions which may not have a krb5 context (although I haven't
checked). I think I can just handle these on a case by case basis.
More information about the krbdev