7 ms·
strncpy() is not a safer strcpy() (2012)
- gonmf 9y agoThe article is literally false. strncpy is safer than strcpy. The argument of the author is it still has room for abuse.
- Gracana 9y agoWhen it fails it leaves you with something that's not a string. It's not safe and it's not a strcpy().
- mikeash 9y agoWhen strcpy fails it results in undefined behavior. strncpy is safer. It is not safe. This article really ought to apply s/safer/safe/ and then it'll all be good.
- geofft 9y agoWhen strncpy fails it leaves you with a character array that isn't NUL-terminated. Passing that to another string-handling function is undefined behavior, because it is not a string.
- mikeash 9y agoRight, so you need to make two mistakes before you get undefined behavior with strncpy, and only one mistake to get it with strcpy. Thus, safer, but not safe.
- sirclueless 9y agoI don't agree. Any input string that violates the strcpy invariants (must be a valid null-terminated string with length <= destination buffer size) will also violate the strncpy invariants if you subsequently treat the destination buffer as a string. Since treating the destination buffer as a string is very tempting to do given that it came from a function named "str*cpy()", you've basically added another "Gotcha" to strcpy() without gaining any safety. You're basically trading one form of undefined behavior that's explicit in strcpy for another form of undefined behavior when a developer mishandles the return value of strncpy.
- mikeash 9y agoI don't get how that adds another gotcha. The scenarios in which strncpy gets you are a strict subset of the scenarios in which strcpy gets you.
- taeric 9y agoIt is the passing of the string that is undefined. You know exactly what you get from strncpy. If you thought it did something else, you were wrong. But it is fully defined. This would be like claiming that malloc is unsafe because it doesn't return a null terminated string.
- dllthomas 9y ago> This would be like claiming that malloc is unsafe because it doesn't return a null terminated string. Well, no, because this behavior is very close to the more commonly desired behavior and often indistinguishable therefrom. That makes it more of a "gotcha".
- dom0 9y agoThat's the difference between bad API design and good API design. strncpy is bad. But so are many APIs that originated in a time where arrays were zero-indexed to save one or two instructions in a compiler. (Though these are a bit younger)
- dllthomas 9y ago> That's the difference between bad API design and good API design. strncpy is bad. But so are many APIs Agreed. > arrays were zero-indexed to save one or two instructions in a compiler Actually, I find zero-indexing more consistent whenever you need to do math with the indexes.
- taeric 9y agoIs it? I expect there are many that think they can malloc a string and immediately pass it to a function to play with. I've seen it done, back when I was a TA. And thinking that strncpy will enforce a null at the end actually seems nonsensical to me. Now. I agree that it may not have at some point in my life. But this is again arguing that it is not safer than strcpy. It most certainly is. Just not completely safe. Which again, is no surprise. Why not point out that it does no checking that you passed it pointers you are allowed to write into? I mean, I get that mistakes can be made. I even agree it is not safe. However, to claim it is not safer is akin to the people that claim Java is not safer than C because example.
- mjevans 9y agoYou called it wrong. It's a primitive and gives you choices. You can either: A) call it with sizeof(target) - 1 to explicitly tell it to NOT over-write the guard byte at the end, OR B) you can add an explicit operation to always over-write the byte at the end
- ori_b 9y agoStrncpy 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 9y agoExcerpting 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.
- dllthomas 9y agoActually, I think it's true that "If you swap out every use of strcpy for a correct use of strncpy, you'll have fewer bugs." Any use of strncpy that breaks would also have broken if strcpy had been used instead, and there exist some situations where strncpy would not break while strcpy would; for instance, if we just inspect a prefix of the resulting string. That said, it's certainly the case that most breaking uses of strcpy, if naively replaced with strncpy, still break.
- geofft 9y ago> Actually, I think it's true that "If you swap out every use of strcpy for a correct use of strncpy, you'll have fewer bugs." Yes, I agree that this statement is true. But my claim is that this is not what the article means by "safer", and the article's definition of "safer" is more useful. That is, people who are saying "The article is wrong because it is possible to use strncpy correctly in cases where strcpy was not being used correctly" are not disagreeing with the article so much as talking past it.
- dnautics 9y agoYou 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
- mjevans 9y agoOnly it isn't. strncpy will /terminate/ at the end of the string if it's shorter than the allocated buffer. If, somehow, the source string is invalid it will then terminate at the specified cutoff point. However one should argue that if it has overflowed bounds, that if the source isn't a valid string, you've already violated the contract of a secure environment and thus something has gone wrong and should be handled at a level higher than string copy preferences. Use of strncpy, however, assumes that the source string /is/ valid, but might overflow the target buffer. In /that/ case truncation may be desired and if not the program flow reacting accordingly is probably desired. (This is C, you don't /throw/ errors, you write explicit paths.)
- geofft 9y agoThere 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/ http://bstring.sourceforge.net/ or with third-party frameworks that include string handling like GLib (https://developer.gnome.org/glib/stable/glib-Strings.html 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 9y agoIf 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 9y agoDoS is way less severe than RCE caused by buffer overflows.
- pcwalton 9y agoI 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.
- masklinn 9y ago> 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 9y agoI'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?
- gpderetta 9y agoThe C standard document N1967 has an overview of pros and cons of annex K and concludes suggesting deprecation or outright removal.
- brynet 9y agoTodd 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 https://www.sudo.ws/todd/papers/strlcpy.html (USENIX '99) Further reading, including EXAMPLEs: http://man.openbsd.org/strlcpy http://man.openbsd.org/strlcpy http://cvsweb.openbsd.org/cgi-bin/cvsweb/~checkout~/src/lib/libc/string/strlcpy.c?rev=1.15&content-type=text/plain 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-implications-of-strncpy/ https://blog.mozilla.org/nfroyd/2013/11/05/the-performance-i...
- fnj 9y agoBe 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.
- brynet 9y agoNoted above; thanks!
- fnj 9y agoAny halfway competent C programmer knows very well the caveat with strncpy. If you tell it to copy 5 characters, and the source has no NULL appearing within those 5 characters, what would you have it do? Copy more than the number you told it, so it could append a NULL? Copy one less than the number you told it, so it would have room to append a NULL? The article is shit.
- Rusky 9y ago> Copy one less than the number you told it, so it would have room to append a NULL? Yep. Or perhaps return an error.
- ordu 9y agoI 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.
- searchfaster 9y agoI have always been using snprintf
- krylon 9y agoHehe, me too. Or memcpy and handling the terminating nul byte myself. But snprintf is just too convenient.
- defined 9y agoThis 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 9y agoThis 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.
- defined 9y agoThat's not unsafe, which is what the article is about.
- torg 9y agoBut as an inexperienced C developer, I found it quite helpful.
- defined 9y ago> But as an inexperienced C developer, I found it quite helpful. Fair enough. I didn't mean to come across as arrogant; I just wasn't thinking about the audience. I'm happy you got value from it.
- deleted 9y ago[deleted]
- hedora 9y agoI 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 9y agoYour 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.
- 0x09 9y agoThe code copies the origin string up to and including the null terminator, which must exist as strlen was used to obtain the size.
- xyzzy4 9y agoIt doesn't copy the null terminator because it's using memcpy. It's only copying the string without the null byte.
- detaro 9y agoI think you are missing the +1 behind the call to strlen. strlen reports the number of chars before the zero-byte, adding one includes it.
- hedora 9y agoI go back and forth between putting the +1 on the first line, or repeating it in the other lines. Putting it on the first line is harder to screw up, but harder to read. For real programs you basically have to put it in the initial computation, or it will be forgotten somewhere later (maybe in a later commit).
- deleted 9y ago[deleted]
- ktRolster 9y agoHere 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 9y agoThat still has poor behavior if newBuf is much larger than oldBuf.
- monochromatic 9y agoIt leaves the small problem and fixes the big problem.
- ktRolster 9y agoWhat poor behavior? Presumably oldBuf is null-terminated, so there will be no buffer overflow (if your strings aren't null-terminated, you have other problems)
- tarpherder 9y agoWhat if size becomes zero?
- deleted 9y ago[deleted]
- anderskaseorg 9y agoIt’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 9y agoThis 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.
- aviggiano 9y agoWhy don't people just use redis' sds (https://github.com/antirez/sds https://github.com/antirez/sds) string library instead of using str* functions? It's much safer and much higher level.