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

[Greg Hudson]

On Wed, 2008-10-15 at 21:58 -0500, Nicolas Williams wrote:
> You use a precision field for %s?  Where?

Not often.  Precision fields:

./src/kadmin/ktutil/ktutil_funcs.c:     sprintf(promptstr, "Password for %.1000s", princ_str);
./src/kadmin/cli/kadmin.c:      sprintf(prompt1, "Enter password for principal \"%.900s\"",
./src/kadmin/cli/kadmin.c:              "Re-enter password for principal \"%.900s\"",
./src/kadmin/cli/kadmin.c:      sprintf(prompt1, "Enter password for principal \"%.900s\"",
./src/kadmin/cli/kadmin.c:              "Re-enter password for principal \"%.900s\"",

The precision field in those uses can probably be removed if the calls
are switched to use snprintf.

We use field widths with %s in 33 places, but it's all printf and
fprintf, not sprintf.  Mostly in plugins/kdb.

> They are quite common now though.  I'll look at the alleged difference
> between Solaris' and BSD' strlcat().

Wikipedia says this:

        there are implementation differences between the BSD and Solaris
        implementations (the return value of strlcat when there is no
        nul in the destination buffer)

A destination buffer containing no nul character would be invalid input
to the strlcat command.  This is no more worrisome than, say, memcpy()
having unspecified behavior when the source and destination buffers
overlap.

On another note, I have done a spot check of the uses of strcpy and
strcat in the krb5 source tree, and a great many of them appear to be
easily replaced with either strdup() or asprintf(), reducing the code
size in the process.  There are some exceptions, but it might be easier
to decide on an approach for the exceptions after eliminating the
common, easy cases.