top | item 14227586

strncpy() is not a safer strcpy() (2012)

32 points| beefhash | 9 years ago |the-flat-trantor-society.blogspot.com

66 comments

order
[+] brynet|9 years ago|reply
Todd C. Miller's original paper, strlcpy and strlcat, is a good read about the issues with strncpy and unbounded strcpy.

https://www.sudo.ws/todd/papers/strlcpy.html (USENIX '99)

Further reading, including EXAMPLEs:

http://man.openbsd.org/strlcpy

http://cvsweb.openbsd.org/cgi-bin/cvsweb/~checkout~/src/lib/...

Barring quibbles about return values, strlcpy is essentially:

    snprintf(dst, sizeof(dst), "%s", src);
A lot of people have been misled by strncpy, when what they really needed was strlcpy:

https://blog.mozilla.org/nfroyd/2013/11/05/the-performance-i...

[+] fnj|9 years ago|reply
Be aware that the strlcpy paper's timing data comes from another era altogether; an era in which a 25 MHz Motorola 68040 and a 166 MHz Alpha had some slight relevance. Nowadays your phone has 10 to 100 times as much performance. Worrying about a few milliseconds then is now wasting brain cells worrying about microseconds.
[+] hedora|9 years ago|reply
I haven't seen anyone actually use the strn* functions correctly, but I have seen correct uses of the str* variants.

Put another way, the following either maintains the string invariant and does not silently truncate out, or it fails on a null pointer dereference. If you forget the + 1, it will certainly fail in valgrind when out is used, and will almost certainly crash your unit tests.

size_t len = strlen(in)+1;

char* out = malloc(len);

memcpy(out, in, len);

Open challenge: Produce something that is correct (i.e., it cannot silently corrupt the string), is as simple, and is less error prone with the strn* functions.

[+] xyzzy4|9 years ago|reply
Your code will only work if malloc() pads with 0s. Otherwise it will be missing the null character at the end of the out string. So I would switch it to calloc() which is guaranteed to pad with 0s. Or use memset to set it all to 0s.
[+] masklinn|9 years ago|reply
> TODO: Discuss the bounds-checking versions added in Annex K of the 2011 ISO C standard

Not of any use, few libcs support Annex K, which essentially "standardises" Microsoft extensions. Annex K was repeatedly rejected by GNU libc.

You're probably better off using the portable BSD functions (strl*).

[+] camgunz|9 years ago|reply
I've wondered about Annex K for a while. Basically as soon as C11 was standardized, every C programmer (including me) said, "oh, Annex K is useless". Is the C standardization process that detached from the C programming community? Or are there real use cases for Annex K that I just can't see?
[+] anderskaseorg|9 years ago|reply
It’s often implicitly assumed in these kind of discussions that the “safe” behavior would be to always create a truncated NUL-terminated string. That is still crazy! Do you want your data to be silently truncated?

The only safe thing to do with a string that doesn’t fit in the available space is to abort with an error. Check those return values!

For this usage, strncpy is perfectly suitable (though the performance concern may still slightly favor another approach):

if (strncpy(dst, src, size) >= size) { abort(); }

[+] dllthomas|9 years ago|reply
This depends a whole lot on context. There are lots of places where silent truncation is the correct behavior.

I don't disagree that return values should be checked, or that things should maybe be more explicit if you want truncation, but calling aborting "the only safe thing" is wrong.

[+] geofft|9 years ago|reply
There is only one thing that is "safe": use a string type that explicitly tracks length (and a string-buffer type that tracks both length and capacity) as a single object, where you're never left guessing the length given a pointer. C strings, which are NUL-terminated, are inherently unsafe. strlcpy() makes different tradeoffs from strncpy(), but isn't "safe" in an absolute sense, either.

As a bonus, your strings can now contain interior NUL bytes, as strings in just about any non-C language can, and so you get improved interoperability.

You can do this in C, with various third-party string libraries like http://bstring.sourceforge.net/ or with third-party frameworks that include string handling like GLib (https://developer.gnome.org/glib/stable/glib-Strings.html). You can also do this in easily-C-compatible languages like C++ or Objective-C, or less C-like compiled languages like Go or Rust or Ada, or other languages like Java / other JVM languages (which will often give you faster performance than C), Python, Ruby, C#, the works.

The only good reason to write code in C that uses C strings is compatibility with some existing code that requires it (either you're making minimal changes to an existing codebase, or you're writing a small amount of code that interoperates with an API that uses C strings). You can't have both safety for large-scale programs and C strings.

[+] mortehu|9 years ago|reply
If you have to handle malicious or potentially buggy input, you probably have to check length anyway to avoid DoS, so std::string isn't automatically safe.
[+] pcwalton|9 years ago|reply
I don't know why this was downvoted. This is the C equivalent of the "if this is news to you, just use NaCl" advice that experts give on articles about pitfalls in unauthenticated AES or whatever. For most people, your advice is the correct advice.
[+] gonmf|9 years ago|reply
The article is literally false. strncpy is safer than strcpy. The argument of the author is it still has room for abuse.
[+] ori_b|9 years ago|reply
Strncpy does not return a valid string. It is not a substitute for strcpy.

Take the strlcpy() function from OpenBSD and incorporate that into your source if you don't have access. Strlcpy() is a well designed substitute for strcpy.

[+] geofft|9 years ago|reply
Excerpting from the article:

> Now having a function like this in the standard library isn't such a bad thing in itself. It's designed to deal with a specialized data structure ...

> The problem is that the name strncpy() strongly implies that it's a "safer" version of strcpy(). It isn't. ....

> It's because strncpy()'s name implies something that it isn't that it's such a trap for the unwary. It's not a useless function, but I see far more incorrect uses of it than correct uses. This article is my modest attempt to spread the word that strncpy() isn't what you probably think it is.

This argument seems correct to me. "Safer," here, does not mean "If you swap out every use of strcpy for a correct use of strncpy, you'll have fewer bugs." As the author is defining it, "safer" includes the risk that someone will be less rigorous with their code by assuming strncpy will solve all their problems. It won't. It will solve some of their problems, yes, but they still have to analyze their problem almost as rigorously as if they were using strcpy.

If a code reviewer cares deeply about strcpy and less deeply about strncpy (and anecdotally this is a thing code reviewers do), then in practice, the use of strncpy is not safer.

[+] dnautics|9 years ago|reply
You should read the article. The issue is not that strncpy is not safer, the issue is that strncpy is not a string operation, but a memory operation
[+] Gracana|9 years ago|reply
When it fails it leaves you with something that's not a string. It's not safe and it's not a strcpy().
[+] ktRolster|9 years ago|reply
Here is one way to handle strncpy() that works in a lot of cases:

strncpy(newBuf, oldBuf, size); newBuf[size-1] = 0;

That way you always end up with a null. It adds the null even when you don't need it, but that's more efficient than adding an 'if' statement.

[+] a_t48|9 years ago|reply
That still has poor behavior if newBuf is much larger than oldBuf.
[+] searchfaster|9 years ago|reply
I have always been using snprintf
[+] krylon|9 years ago|reply
Hehe, me too. Or memcpy and handling the terminating nul byte myself. But snprintf is just too convenient.
[+] fnj|9 years ago|reply

[deleted]

[+] ordu|9 years ago|reply
I agree, but I'd clarify a little. Its not a caveat with strncpy, its caveat with string representation: to work with null-terminated strings you needs to remember that maximum length of string is lower than size of an array. For example you should remember it when allocating memory, when concatenating and calculating size. It can be named as caveat, though not with strncpy, but more general caveat with strings. It needs some training to master strings in C, but any textbook has explanations and exercises. So, you a right, the article is shit.
[+] Rusky|9 years ago|reply
> Copy one less than the number you told it, so it would have room to append a NULL?

Yep. Or perhaps return an error.

[+] defined|9 years ago|reply
This is not news to experienced C developers.

All it takes to avoid a missing NUL terminator is something like

  #define safer_strncpy(dest, src, n) \
    do { \
        if (n > 0) \
            strncpy(dest, src, n)[n - 1] = '\0'; \
    } while (0)
[+] jwilk|9 years ago|reply
This still appends null bytes to fill the whole buffer, which is almost never what you want.

Use snprintf() or strncat(), as suggested by the article.

[+] torg|9 years ago|reply
But as an inexperienced C developer, I found it quite helpful.