"Secure coding" audit checkers and Kerberos

[Nicolas Williams]

On Wed, Oct 15, 2008 at 01:00:30PM -0700, Love Hörnquist Åstrand wrote:
> 15 okt 2008 kl. 11:54 skrev Greg Hudson:
> 
> >buflen = strlen(s1) + strlen(s2) + strlen(s3) + 1;
> >buf = malloc(buflen);
> >if (!buf) return ENOMEM;
> >(void) strlcpy(buf, s1, buflen);
> >(void) strlcat(buf, s2, buflen);
> >(void) strlcat(buf, s3, buflen);
> 
> asprintf(&buf, "%s%s%s", s1, s2, s3);
> if (buf == NULL)
>   return ENOMEM;

And if you don't want to malloc() ('cause then you have to remember to
free) then:

	char dst[MAX_SOMETHING_OR_OTHER];

	...
	if (strlcpy(dst, s1, sizeof (dst)) >= sizeof (dst))
		return (KRB5_ERR_SOMETHING_IS_TOO_LONG);
	if (strlcat(dst, s2, sizeof (dst)) >= sizeof (dst))
		return (KRB5_ERR_SOMETHING_IS_TOO_LONG);
	if (strlcat(dst, s3, sizeof (dst)) >= sizeof (dst))
		return (KRB5_ERR_SOMETHING_IS_TOO_LONG);

While we're at it, I am concerned about leaks and double-frees,
particularly the latter (there have been enough of those).

I'd really like to have every function in MIT krb5 that mallocs anything
(or calls any function that does, or which ...) coded in a consistent
way.

Preferably:

    All pointer variables should be initialized to NULL where they are
    declared.  Ungated free()s should be used if that is portable, else
    just if (variable != NULL) free(variable).  Use a single return at
    the bottom of the function, with a goto for any failures.

Nico
--