"Secure coding" audit checkers and Kerberos

ghudson@MIT.EDU 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:

  sprintf
  strcpy/strcat
  sscanf/fscanf
  random/lrand48/rand

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
  snprintf.
* 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 mailing list