On Tuesday, 9 May 2017 at 06:15:12 UTC, H. S. Teoh wrote:
On Mon, May 08, 2017 at 06:33:08PM +0000, Jerry via
Digitalmars-d wrote:
strncpy(buf, src, sizeof(buf));
Quick, without looking: what's wrong with the above line of
code?
Not so obvious, huh? The problem is that strncpy is, in spite
of being the "safe" version of strcpy, badly designed. It does
not guarantee buf is null-terminated if src was too long to fit
in buf! Next thing you know -- why, hello, unterminated string
used to inject shellcode into your "secure" webserver!
The "obvious" fix, of course, is to leave 1 byte for the \0
terminator:
strncpy(buf, src, sizeof(buf)-1);
Except that this is *still* wrong, because strncpy doesn't
write a '\0' to the end. You have to manually put one there:
strncpy(buf, src, sizeof(buf)-1);
buf[sizeof(buf)-1] = '\0';
The second line there has a -1 that lazy/careless C coders
often forget, so you end up *introducing* a buffer overrun in
the name of fixing another.
This single problem area (improper use of strncpy) accounts for
a larger chunk of code I've audited than I dare to admit -- all
just timebombs waiting for somebody to write an exploit for.
Adding to that, strncpy() is also a performance trap. strncpy
will not stop when the input string is finished, it will fill the
buffer up with 0.
so
char buff[4000];
strncpy(buff, "hello", sizeof buff);
will write 4000 bytes on every call.
The thing with strncpy() is that it's a badly named function. It
is named as a string function but isn't a string function. Had it
been named as memncpy() or something like that, it wouldn't
confuse most C programmers. If I get my C lore right, the
function was initially written for writing the file name in the
Unix directory strncpy(dirent, filename, 14); or something like
that.
