
strncpy() is not a safer strcpy() (2012) - beefhash
https://the-flat-trantor-society.blogspot.com/2012/03/no-strncpy-is-not-safer-strcpy.html
======
brynet
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](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/...](http://cvsweb.openbsd.org/cgi-
bin/cvsweb/~checkout~/src/lib/libc/string/strlcpy.c?rev=1.15&content-
type=text/plain)

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...](https://blog.mozilla.org/nfroyd/2013/11/05/the-performance-
implications-of-strncpy/)

~~~
fnj
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_.

~~~
brynet
Noted above; thanks!

------
hedora
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
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.

~~~
0x09
The code copies the origin string up to and including the null terminator,
which must exist as strlen was used to obtain the size.

~~~
xyzzy4
It doesn't copy the null terminator because it's using memcpy. It's only
copying the string without the null byte.

~~~
detaro
I 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
I 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).

~~~
masklinn
> Putting it on the first line is harder to screw up, but harder to read.

Mayhaps "len" should be renamed to something clearer?

------
masklinn
> 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
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?

~~~
gpderetta
The C standard document N1967 has an overview of pros and cons of annex K and
concludes suggesting deprecation or outright removal.

------
anderskaseorg
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
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
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/](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
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
DoS is way less severe than RCE caused by buffer overflows.

------
gonmf
The article is literally false. strncpy is safer than strcpy. The argument of
the author is it still has room for abuse.

~~~
geofft
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.

~~~
dllthomas
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."

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
> _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.

~~~
dllthomas
Fair enough.

------
aviggiano
Why 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.

------
ktRolster
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
That still has poor behavior if newBuf is much larger than oldBuf.

~~~
ktRolster
What 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)

------
searchfaster
I have always been using snprintf

~~~
krylon
Hehe, me too. Or memcpy and handling the terminating nul byte myself. But
snprintf is just too convenient.

------
defined
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
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.

~~~
defined
That's not unsafe, which is what the article is about.

