security goals re strcpy/strcat/sprintf (Re: "Secure coding" audit checkers and Kerberos)

[Tom Yu]

ghudson at MIT.EDU writes:

> 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.

This thread has generated a lot of traffic, and I would like to
refocus our attention on a subset of the topics that have arisen.

I would like to clarify why we would like to eliminate or reduce our
usage of "unsafe" string functions such as strcpy, strcat, and
sprintf.  While we have striven to make safe use of these these
functions by coding explicit length checking in the surrounding code,
some static code analysis tools flag these uses as problems.

When these sorts of false positive reports arise, they require us to
somehow ensure that the presumed-correct length checking code is
actually correct.  If this length checking code is widely dispersed
throughout the source code, such manual validation can become very
time-consuming and error-prone.

I acknowledge that the "unsafe" string functions can be used safely
and that "safe" string functions can be used unsafely.  Regardless, I
think there is a meaningful difference between these "safe" and
"unsafe" functions.  It is easier to determine whether a particular
use of "safe" function is actually safe than it is to determine
whether a particular use of an "unsafe" function is safe, particularly
if the pieces of compensating length checking code, etc. are
replicated everywhere the "unsafe" function is used.  The
vulnerabilities resulting from unsafe uses of the "safe" functions,
which could be truncation-related problems, are less severe than those
vulnerabilities resulting from unsafe uses of the "unsafe" functions,
which are more likely to be buffer overflow type problems.

Overall, I think we will end up replacing uses of the "unsafe" string
functions with strdup, asprintf, and snprintf.  For widely-dispersed
and repeated code patterns, we should consider making utility
functions to reduce the incidence of programmer error.  Some thoughts
I've had on the subject, taking into account some of the recent
conversation, are below.

For replacement candidates that allocate memory, we could consider:

* strdup

* asprintf

For replacement candidates that write into fixed-size buffers, we
could consider:

* snprintf

* strlcpy

* strlcat

* memcpy

snprintf is standard (C99, SUSv3), but some implementations are known
to have (hopefully well-characterized by now) problems.

asprintf is not standard, but is trivially implementable in terms of
snprintf.  (as we have already done)

strlcpy/strlcat are not standard, and I recall reading allegations
that their behavior is not very consistent across implementations.

memcpy is standard, but using it with null-terminated strings requires
additional calls to strlen and such, which can further complicate
automatic or manual inspection.

Known issues with the printf family on Solaris include some
"interesting" interpretations of the precision field for %s
specifiers, such as counting "column width" rather than bytes.  This
can make dealing with gss_buffer_t and other such explicit-length
string-like data structures problematic, depending on the current
locale.  Nico or other Sun folks, any thoughts on this?

Also, I suggest that we refine the description of our supported
platforms to include some qualifiers like "includes an implementation
of snprintf that is conforming or that is non-conforming in one of a
small number of well-characterized ways".

-- 
Tom Yu
Development Manager
MIT Kerberos Consortium