Why Is Strncpy Insecure

Why is strncpy marked as unsafe?

strncpy has a few dangerous quirks.

First, it zeros the target buffer past the end of the copy, which can be surprising.

Second, if there is not enough room in the target buffer, it does not null terminate the target buffer.

Third, if it truncates, it 'mostly works'. Which discourages error handling (truncated strings are often worse than useless, but do not appear to be worse than useless at first glance).

strncpy_s requires an input length (or explicit truncation request), and errors if there is not enough room to null terminate (writing just a zero length string in the output). The input length is sometimes inefficient to provide (and not required for some of its changes), but it does guarantee a null terminated output buffer (so long as it isn't a nullptr, or zero length) even in error conditions. I am unsure if it zeros past the end of the copied string or not.

This behavior prevents or mitigates some common fenceposting errors in string code.

Why is strcpy unsafe in C?

strcpy has no way of knowing how large the destination buffer is (i.e. there is no length parameter) so sloppy programming using it can lead to overrunning the buffer and corrupting other memory. Such an overrun can lead to crashes, odd behaviour and may be exploitable by malware authors.

BTW, look at strncpy that takes a length parameter. One problem to be aware of is that strncpy will not terminate the string if the buffer is too small so it is dangerous in its own way.

In answer to your comment/question - it will depend on context. If you know the buffer is large enough (for example, you allocated it in the previous line at the correct size), use strcpy. If there is any possibility of an overflow then use strncpy and make sure you set the last position in the buffer to null. Note that "any possibility" should be interpreted very generously - unless the previous five or six lines are enough to show safety then use strncpy.

Also see Andon's comment above about strncpy_s. Finally, if you do use strcpy, you might want to add a #pragma to suppress the warning at that location.

Why should you use strncpy instead of strcpy?

strncpy combats buffer overflow by requiring you to put a length in it. strcpy depends on a trailing \0, which may not always occur.

Secondly, why you chose to only copy 5 characters on 7 character string is beyond me, but it's producing expected behavior. It's only copying over the first n characters, where n is the third argument.

The n functions are all used as defensive coding against buffer overflows. Please use them in lieu of older functions, such as strcpy.

Why strncpy() is not respecting the given size_t n which is 10 in temp2?

The strncpy function is respecting the 10 byte limit you're giving it.

It copies the first 10 bytes from string to temp2. None of those 10 bytes is a null byte, and the size of temp2 is 10, so there are no null bytes in temp2. When you then pass temp2 to printf, it reads past the end of the array invoking undefined behavior.

You would need to set the size given to strncpy to the array size - 1, then manually add the null byte to the end.

strncpy(temp2, temp1, sizeof(temp2)-1);
temp2[sizeof(temp2)-1] = 0;

Why is this code vulnerable to buffer overflow attacks?

On most compilers the maximum value of an unsigned short is 65535.

Any value above that gets wrapped around, so 65536 becomes 0, and 65600 becomes 65.

This means that long strings of the right length (e.g. 65600) will pass the check, and overflow the buffer.

Use size_t to store the result of strlen(), not unsigned short, and compare len to an expression that directly encodes the size of buffer. So for example:

char buffer[100];
size_t len = strlen(str);
if (len >= sizeof(buffer) / sizeof(buffer[0]))  return -1;
memcpy(buffer, str, len + 1);