PR 81117 asks for improved detection of common misuses(*) of
strncpy and strncat. The attached patch is my solution. It
consists of three related sets of changes:
1) Adds a new option, -Wstringop-truncation, that diagnoses calls
to strncpy, and stpncpy (and also strncat) that truncate the copy.
This helps highlight the common but incorrect assumption that
the first two functions NUL-terminate the copy (see, for example,
CWE-170) For strncat, it helps detect cases of inadvertent
truncation of the source string by passing in a bound that's
less than or equal to its length.
2) Enhances -Wstringon-overflow to diagnose calls of the form
strncpy(D, S, N) where the bound N depends on a call to strlen(S).
This misuse is common in legacy code when, often in response to
the adoption of a secure coding initiative, while replacing uses
of strcpy with strncpy, the engineer either makes a mistake, or
doesn't have a good enough understanding of how the function works,
or does only the bare minimum to satisfy the requirement to avoid
using strcpy without actually improving anything.
3) Enhances -Wsizeof-pointer-memaccess to also warn about uses of
the functions to copy an array to a destination of an unknown size
that specify the size of the array as the bound. Given the
pervasive [mis]use of strncpy to bound the copy to the size of
the destination, instances like this suggest a bug: a possible
buffer overflow due to an excessive bound (see, for example,
CWE-806). In cases when the call is safe, it's equivalent to
the corresponding call to memcpy which is clearer and can be
more efficient.
Martin
PS By coincidence rather than by intent, the strncat warnings
are a superset of Clang's -Wstrncat-size. AFAICS, Clang only
warns when the destination is an array of known size and
doesn't have a corresponding warning for strncpy.
[*] Here's some background into these misuses.
The purpose of the historical strncpy function introduced in V7
UNIX was to completely fill an array of chars with data, either
by copying an initial portion of a source string, or by clearing
it. I.e., its purpose wasn't to create NUL-terminated strings.
An example of its use was to fill the directory entry d_name
array (dirent::d_name) with the name of a file.
The original purpose of the strncat function, on the other hand,
was to append a not necessarily NUL-terminated array of chars
to a string to form a NUL-terminated concatenation of the two.
An example use case is appending a directory entry (struct
dirent::d_name) that need not be NUL-terminated, to form
a pathname which does.
Largely due to a misunderstanding of the functions' purpose they
have become commonly used (and misused) to make "safe," bounded
string copies by safeguarding against accidentally overflowing
the destination. This has led to great many bugs and security
vulnerabilities.
