Difference between revisions of "Coding style/Practices"

Revision as of 15:33, 23 October 2008

String Handling

Code should not use any of the following functions: strcpy, strcat, sprintf, or any *scanf function.

Dynamically allocated buffers are preferred to fixed-sized buffers. When using dynamic buffers:

• Use strdup or asprintf for simple constructions.
• (NOT YET IMPLEMENTED) Use the k5buf module for complex constructions. If this is not desirable, strlcpy and strlcat are valid alternatives.

When using fixed-sized buffers:

• (NOT YET IMPLEMENTED) Use strlcpy for simple copies.
• Use snprintf for simple compound constructions. Avoid using precision or field width specifiers in snprintf format strings.
• To check for truncation when using snprintf, use the following approach:

result = snprintf(buffer, sizeof(buffer), "format string", ...);
if (SNPRINTF_OVERFLOW(result, sizeof(buffer))
    handle_overflow_error;

The SNPRINTF_OVERFLOW macro is defined in k5-platform.h.

• (NOT YET IMPLEMENTED) Use the k5buf module for complex constructions. If this is not desirable, strlcpy and strlcat are valid alternatives.

Current conformance

Existing code does not conform. The build system does not yet ensure that strlcpy and strlcat are available. The k5buf module is not yet implemented or documented.

Rationale

Audit tools such as lint and Coverity flag all uses of strcpy and similar functions, even if they are used safely. Verifying that the function is used safely requires manual inspection. Using safer alternatives does not guarantee code safety, but does reduce the likelihood of a catastrophic buffer overflow vulnerability.

In some compilation environments under Solaris, field widths and precisions are computed using screen columns rather than bytes.

sprintf returns a signed integer; buffer sizes are typically size_t (which is an unsigned type). Comparing the two directly will result in a warning from some compilers, and has unpredictable semantics depending on the relative widths of int and size_t. Also, some sprintf implementations, such as the one in Solaris prior to version 10, return -1 on a buffer overflow, whereas the C99 behavior returns the number of bytes which would have been written to the string. The SNPRINTF_OVERFLOW macro uses casts to address both issues.