Beware of strncpy() and strncat()

Most C programmers will instantly recognize the idioms for memcpy():

// OK: copy data into a buffer
char dest[64];
memcpy(dest, src, sizeof(dest));

// OK: as above, but using a struct
complex_t foo;
memcpy(&foo, &bar, sizeof(foo));

Likewise with memset():

// OK: clear a buffer
char buf[128];
memset(buf, 0, sizeof(buf));

Most C programmers also know to avoid the legacy strcpy() and strcat() functions, as these commonly introduce buffer-overflow problems. Instead, C programmers should use the newer strncpy() and strncat() functions, which check the size of the buffer they’re copying data into.

It is tempting to use the string functions like memory functions, as they have nearly identical APIs:

// prototype for memcpy
void* memcpy(void *dest, const void *src, size_t n);

// prototype for strncpy
char* strncpy(char *dest, const char *src, size_t n);

While the size parameter n to both functions means the same thing, it’s not safe to use the functions the same way. Consider this incorrect function:

// XXX: do not do this
char dest[8];
strncpy(dest, src, sizeof(dest));

Let’s say this is run when src is "super". The full string will be copied, and dest will be null terminated after the call, just as expected.

There’s a problem if instead the function is called when src is "supercalifragilisticexpialidocious". This string is too long for the buffer, so only the first 8 bytes will be copied. However, this afterwards dest is not null-terminated. This means that if dest is passed to a function that expects a C-style null-terminated string, that function will probably read past the end of the buffer. This isn’t exactly a buffer overflow as it involves a read rather than a write. But it’s still a serious issue as it can cause other kinds of memory corruption issues.

The correct idiom with strncpy() is the following:

// OK: correctly copy src
char dest[8];
strncpy(dest, src, sizeof(dest) - 1);

The strncat() function has the exact same issue. For example:

// OK: good so far
char *buf = malloc(BUF_SIZE);
strncpy(buf, default_value, BUF_SIZE - 1);

// XXX: after this buf may not be null-terminated
strncat(buf, another_buffer, BUF_SIZE - strlen(buf));

Instead you should copy one less byte (or allocate one more):

// OK: ensure buf is null-terminated after concatenation
strncat(buf, another_buffer, BUF_SIZE - strlen(buf) - 1);

GCC will automatically warn with the preceding examples with -Wstringop-truncate. Even better is to use -Wall which implies this flag and various others checks for string misuse.