Strncpy() is not a “safer” strcpy()

http://the-flat-trantor-society.blogspot.com/2012/03/no-strncpy-is-not-safer-strcpy.html

by luu • 11 years ago

56 70 11 years ago

the-flat-trantor-society.blogspot.com

lawl2• 11 years ago

tl;dr: strncpy() doesn't null terminate, but doesn't mention strlcpy()

http://www.openbsd.org/cgi-bin/man.cgi?query=strlcpy

masklinn3• 11 years ago

Because, much like reallocarray, strlcpy is not part of the C standard library.

The correct solution, assuming you can't just import libbsd or include its functions, is probably to use snprintf. It goes guarantee null-termination of the target. So

    strncpy(dst, src, size_dst);

becomes

    snprintf(dst, size_dst, "%s", src);

Checking for partial copy is not ideal though, snprintf returns the number of bytes it would have copied had the destination buffer been infinite excluding the final NUL byte. So a copy is only complete if `ret < size_dst`.

snprintf is not available in C89 though...

rkangel• 11 years ago

Or just to add an implementation of strlcpy in a util module in your codebase. e.g.: http://mirror.fsf.org/pmon2000/3.x/src/sdk/libc/string/strlc...

jjnoakes4• 11 years ago

snprintf is also slower. It has to parse the format string.

Just use strncpy and NUL-terminate the buffer. It's not hard. Write a two line function to do it for you, and use that function in place of strncpy.

MrBuddyCasino• 11 years ago

I'm not a C programmer, but even I knew that strlcpy() and snprintf() would come up in this discussion. How fucked up is that?

Is there a resource where these issues are answered once and for all, for the novice C programmer? Also, are there deprecation warnings emitted by the compiler for non-safe C stdlib functions?

clarry• 11 years ago

On OpenBSD the linker whines about uses of unsafe functions. I don't see it becoming mainstream though, as there are so many people who disagree about what's unsafe.

lawl2• 11 years ago

Not a C programmer either. But I've seen projects banning strcpy. The code was littered with strncpy(a, b, strlen(b)). So I'm not sure how useful these warnings would be.

cremno• 11 years ago

This article is also on GH (including some changes): https://github.com/Keith-S-Thompson/the-flat-trantor-society...

StavrosK• 11 years ago

That second paragraph means that if the string pointed to by s2 is shorter than n characters, it doesn't just copy n characters and add a terminating null character, which is what you'd expect.

But what I would expect is that it would copy len(s2) characters, not n.

patal• 11 years ago

That's correct. I think the author meant that.

JulianMorrison3• 11 years ago

C people, isn't it perhaps time to quietly pasture the existing standard library and create a new one, from scratch, that isn't chock full of booby traps in the small print?

pjmlp3• 11 years ago

The real problem, how C sees strings, arrays and pointers would still persist.

Implicit conversion between vectors and pointers should have never happened. I don't get why people find so hard to write &vector[0] for those cases where a pointer is needed.

Also if other languages, for other OS, older than C could do bound checking elision, C not doing it was only saving work for the compiler developer.

kazinator4• 11 years ago

If &array[0] were needed, it wouldn't solve anything. The semantics that you can take the pointer to the first element, and pass it around and treat it as an array, with no meta-data connecting it to the original properties of the array, would still be there.

The requirement for a cast would be better, because that's the existing notation for explicitly requesting conversions that are identified as unsafe. I.e.

   char array[42];
   char *p = array; /* error, implicit conversion */
   char *q = (char *) array; /* OK */
   char *s = &array[0]; /* Sneaky: OK, without a cast */

sirwolfgang• 11 years ago

I mean, that pretty much is the entire premise of C++. Which notably does have a full wrapped string class.

humanrebar• 11 years ago

To be fair, maintaining compatibility with existing software (including libc) is the reason, either directly or indirectly, for many (most?) of the gotchas in C++.

Then again "your C code is valid C++ code!" was perhaps the biggest selling point of C++ when it came out.

anton_gogolev2• 11 years ago

Or, like, dozens and dozens of them, all differing in almost any aspect imaginable.

And no, std::string is decidedly not a proper "string class".

kazinator4• 11 years ago

Every significantly large C program in C's history quietly farms its own library.

101914• 11 years ago

Opinion: Some not so quietly; in any case, most of them are not worth reusing. But there are exceptions, e.g., stralloc.h

Opinion: Personally, because I strive to learn more than to be "productive", I prefer languages that force the author to write her own libraries. As I see it, the alternative is languages that discourage this, effectively coercing the author to use other people's libraries, the quality over which she has no control.

Opinion: For me, the bad part is that there are so many poorly thought out C functions in the wild (even including the "standard" ones); the good part is that the C language encourages authors to write their own libraries.

Given the choice, I still prefer assembly to C. I guess this is because I prefer to write small programs; perhaps I am not smart enough to write large ones.

belorn• 11 years ago

There is one, and it exist inside the gnu libc.

strndup performs identical to strncpy except that it always terminates the destination string. (https://www.gnu.org/software/libc/manual/html_node/Copying-a...)

jpollock• 11 years ago

strndup is not a replacement for strncpy, it allocates memory, whereas strncpy takes an existing buffer. In-place memory usage is very important for preparing anything of a fixed size (structs) to be written out to disk or the network.

JulianMorrison3• 11 years ago

You remember the Holy Grail scene from Indiana Jones? The problem is not the absence of the grail, somewhere on the shelf. The problem is all the others and the fact that if you pick the wrong one, your program desiccates, crumbles to dust, and is blown away by a conveniently dramatic wind.

A grail shelf that wasn't intended as a booby trap would contain one cup, the right one.

lmm2• 11 years ago

Most people who would be in a position to switch to such a library have already migrated to a safer language than C.

pjc504• 11 years ago

Backwards-incompatible change breaks code. And it may do so silently. And the C standardisers don't really have the political clout to do this. Trying to force people to "new C" would be a lot of work and would probably result in a culture split like the backwards-incompatible Perl and Python splits. OpenBSD have managed to move people to strlcpy(), but that's about it.

People are just as likely to migrate to another language (Rust? Go? Even C++) as upgrade the old C code.

PythonicAlpha3• 11 years ago

But it would be possible to create new functions with new names that supersede the old ones, so that the old stdc-lib will be discontinued silently.

PythonicAlpha3• 11 years ago

At least, there are some corners that would need to be addressed. A new, better standard library would provide safer string handling and omit the traps that where left out for years to preserve compatibility.

The C++ standard seems to get much more attention than C. C seems to be the step-daughter of programming languages, but still so many projects rely simply on standard C (and not C++).

tjr• 11 years ago

C was the first programming language I learned well. I've co-written two books on C programming. I've built software ranging from text adventure games to flight management networking software in C.

I personally now avoid C as much as possible.

angry_octet2• 11 years ago

For someone being so pedantic it is puzzling that he doesn't address the actual title of his post. No one claims that strncpy is 'safe' they just say 'safer', that is, less likely than normal strcpy to have catastrophic bugs. And I think that claim stands up: make sure your n in strncpy is the length of your buffer, do a strncpy, write a null at the end of the buffer. Voila, all those strcpyies from unexpectedly large strings writing in your heap are gone. No need to constantly strlen source strings to check the length -- which in itself can cause a segfault if they are not null terminated.

So yes, it is safer. You would be mad not to use it.

unwind• 11 years ago

Replacing one function call with a function call to a function that does almost the right thing in many cases, but potentially wastes time by pointlessly zero-filling a buffer, and an extra assigment statement is not an improvement. You would be mad to do that.

kazinator4• 11 years ago

Starting in C99, you can do this:

  snprintf(buffer, size, "%s", source);

This can be put into a #define macro or inline function.

If what you want is a null terminated, truncated copy that fits into a given buffer size, and snprintf is available, you are mad to still use strncpy.

Also, strncpy pads the destination with nulls, if there is remaining space. Because of this, the evident purpose of strncpy is to fill fixed-width text fields in "old school" fixed-field database records. Writing the extra nulls is wasteful if the data type in question is nothing but a C string.

jedbrown2• 11 years ago

With fixed-width records, writing the extra nulls may be important for data privacy reasons and in any case, it allows comparison using memcpy(3), diff(1), and checksums.

maxlybbert• 11 years ago

The article also suggests using strncat for this: dest[0] = '\0'; strncat(dest, src, size);.

angry_octet2• 11 years ago

I think we can agree that strncpy is not perfect. Perhaps we can rank by levels of sanity?

my_string_cpy() < strcpy < strncpy < strncat < snprintf < strlcat

I dislike the strncat approach because if the prior null termination gets lost then problems ensue, whereas with snprintf that won't happen. The BSD man page mentions that snprintf may not be safe for use in signal handlers on some systems; obviously snprintf is a much bigger piece of code.

Personally I dislike using extensive macros, but I'll leave that for the flame war.

pedrow• 11 years ago

It's worth noting that, quite often when this subject comes up, people suggest a straight replacement with strncpy, strlcpy, snprintf, or something similiar. This is safer because it doesn't overwrite memory or cause a segfault, BUT, that's not always enough. If the code just carries on with the truncated string that's data loss and it might cause trouble later. For example imagine a system designed for 8.3 filenames - if you pass a long filename and it gets truncated it may then refer to a completely different file. The man page for strlcpy (IIRC) reminds you of this.

brucedawson• 11 years ago

People keep on misusing strncpy which is why people keep blogging about it. I wrote about it two years ago after running across one too many abuses:

https://randomascii.wordpress.com/2013/04/03/stop-using-strn...

I tried to give alternatives. Calling strncpy and then manually null-terminating is not a good alternative. Creating a wrapper that does this is better. That wrapper could optionally abort the program on string truncation.

And, probably more importantly, a template wrapper can infer the destination size, thus getting rid of the error-prone specification of the destination size (I have seen it specified incorrectly many times).

You'd be mad to use strncpy when better alternatives are so easy to find or create.

_kst_6• 11 years ago

(I'm the author of the linked blog post.)

strcpy() can be used safely if you ensure that the destination array is big enough to contain the source string. (Yes, that can be non-trivial.)

strncpy() by itself doesn't write past the end of the destination array (assuming you call it correctly) -- but it can easily leave the destination without a terminating null character. This causes undefined behavior if you then pass the destination array to another string function. strncpy() is not dangerous by itself, but it can leave a booby-trap that causes other string functions to be dangerous.

This:

    strncpy(dest, source, size);

can be replaced by this:

    dest[0] = '\0';
    strncat(dest, source, size);

which will leave the destination properly null-terminated.

Note that this can still quietly truncate the string. Ignoring that possibility is rarely the right thing to do.

useerup• 11 years ago

Microsoft has a number of dangerous c functions that are on the Secure Development Lifecycle "banned" list. And yes, strncpy is one of them.

Some insights here: https://msdn.microsoft.com/en-us/library/bb288454.aspx

strncpy example vulnerability experience linked to from that article "Buffer Overflow in Apache 1.3.xx fixed on Bugtraq - the evils of strncpy and strncat!": http://blogs.msdn.com/b/michael_howard/archive/2004/10/29/24...

Source code for MS products are automatically (since SDL) screened for use of the banned functions. You'll have to use the "safer" alternatives.

earlz• 11 years ago

It's a shame that strncpy doesn't have the semantics of OpenBSD's strlcpy, which I think behaves as everyone would expect.

http://www.openbsd.org/cgi-bin/man.cgi?query=strlcpy

gvb• 11 years ago

There is a very simple, efficient solution: always put a null termination character in the end position of the destination after the strncpy(). If the source is longer than the destination, it will properly terminate the destination. If it isn't longer, it didn't cost (hardly) anything.

Unfortunately, it isn't automatic so some times humans will forget to do it.

    #include <stdio.h>
    #include <string.h>

    int main(int argc, char *argv[])
    {
        char *src = "Gobblygook is a long string";
        char dest[5];

        strncpy(dest, src, sizeof(dest));
        dest[sizeof(dest) - 1] = '\0';

        printf("src  = %s\ndest = %s\n", src, dest);
    }

    make test
   ./test
    src  = Gobblygook is a long string
    dest = Gobb

pjc504• 11 years ago

While I've done this myself, it's not a 100% solution, because now you get a truncated string that further processing may not be expecting. For example, if you validate the un-truncated string somehow then take a copy of it, the copy may be invalid.

tobiasu• 11 years ago

This beard has its own beards.

JustSomeNobody• 11 years ago

What do people get out of pedantic rants such as this? Anyone who's written anything in C is quite aware of the issue here.

And then there's this gem:

"...If the source string is 5 characters long, and the target is a 1024-byte buffer, and you set n to the size of the target, strncpy will copy those 5 characters and then fill all 1019 remaining bytes in the target with null characters. Since all it takes to terminate a string is a single null character, this is almost always a waste of time.

Ok, so that's not so bad. CPUs are fast these days, and filling a buffer with zeros is not an expensive operation, right? Unless you're doing it a few billion times, but let's not worry about premature optimization."

Does he think languages with built in String types have some magical optimization juice that make string operations fast? How long does it take to instantiate a new String object, run it's copy constructor, blah blah.

Any C programmer looks to avoid these types of situations.

_kst_6• 11 years ago

What do people get out of pedantic rants such as this? Anyone who's written anything in C is quite aware of the issue here.

Unfortunately, that's not the case.

I presume this link was posted here as a result of my recent comment on the "Don't Learn C the Wrong Way" article, which recommends strncpy() and strlcpy() as safer alternatives to strcpy().

I've see a number of calls to strncpy(). I've rarely seen such calls explicitly null-terminate the destination array. I've even seen things like

    strncpy(dest, src, strlen(src));

which is certainly no safer than strcpy().

What I get out of this particular pedantic rant is an opportunity to let C programmers know that strncpy() isn't what they might thing it is (as well as a mention on the front page of Hacker News!).

_kst_6• 11 years ago

Does he think languages with built in String types have some magical optimization juice that make string operations fast? How long does it take to instantiate a new String object, run it's copy constructor, blah blah.

No, I don't think that. I didn't mention it because I wasn't talking about languages with built-in String types; I was talking about C.

bitwize• 11 years ago

You should be using strcpy_s...

Strncpy() is not a “safer” strcpy()

Related Stories