Stop using strncpy already (2013)

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

by ohjeez • 8 years ago

53 77 8 years ago

randomascii.wordpress.com

dang• 8 years ago

Discussed at the time: https://news.ycombinator.com/item?id=5491121

gp7• 8 years ago

Two years after this post, the Linux kernel added strscpy, the api of which is equivalent to the safe strncpy in this post. Internally, it stops copying once it reaches the null terminator.

https://lwn.net/Articles/659214/

sevensor4• 8 years ago

The article's proposed implementation uses a template to guarantee that egregious mistakes will fail to compile. This is clever, but it's C++. The kernel's version isn't quite as safe because it can overrun the destination. In principle it's impossible for it to be safe, because this is C, and neither the compiler nor the programmer can tell how big the destination is. We have to rely, as the article complains, on the programmer calling the function to know how big the buffer is.

IChooseYou• 8 years ago

Also Microsoft's strncpy_s has been around forever.

brucedawson8• 8 years ago

It's too bad strncpy_s is so unwieldy to use. Having to pass the TRUNCATE flag is frustrating. I really want a two argument (dst, src) function that guarantees null termination.

rurban3• 8 years ago

But Microsoft's strncpy_s is unsafe. My safeclib also had the unsafe versions behind the non-default ./configure --enable-unsafe switch.

But this year I decided to make them safe instead. In this case the spec is broken.

iamthad• 8 years ago

strscpy seems like a nice interface. Is an implementation outside of the kernel available? Preferably one that is permissively-licensed?

Also, it doesn't appear to be specified what happens if the count argument is too large to be represented as a ssize_t. The destination buffer would have to be extremely large, so it probably doesn't happen in practice, but it'd be good to specify it, or at least explicitly state it's unspecified / undefined.

https://www.kernel.org/doc/htmldocs/kernel-api/API-strscpy.h...

ndesaulniers• 8 years ago

I've had patches rejected from the Linux kernel that tried to switch away from strncpy in favor of strlcpy. The behavior that strncpy continues to zero out the rest of the destination in the case where size of source is less than destination was being relied upon to not leak uninitialized memory to userspace. Seems strscpy alone suffers the same.

beefhash• 8 years ago

strlcpy is designed to solve the null-termination problems – it always null-terminates. It’s certainly an improvement over strncpy, however it isn’t natively available in VC++.

Let's not forget that it's also not natively available in glibc. Though you'll find it in every BSD you can think of.

jandrese2• 8 years ago

Sadly, it still has this braindamage:

  Like snprintf(3), the strlcpy() and strlcat() functions return the total
  length of the string they tried to create.  For strlcpy() that means the
  length of src.  For strlcat() that means the initial length of dst plus
  the length of src.

So if you've mmaped in something like the OpenStreetMaps xml dump file and try to strlcpy out the first 100 bytes (because you didn't want to have to null terminate it yourself after a memcpy), you'll notice that your program is running rather slowly.

kevindqc2• 8 years ago

"Let's read all the data you don't want because!" nice

needlepont3• 8 years ago

That is trusting user input. You don't do that, right?

tedunangst• 8 years ago

Don't treat mmapped files as strings.

umvi• 8 years ago

I feel like there are a lot of good alternatives to C. What we did on our team was get our C compiling under g++, and then refactor as C++14. Now string copies are trivial and safe. I'm sure it would have been even safer to switch to Rust or something, but no one in my company has any experience with that, while there were a few C++ gurus.

jstimpfle7• 8 years ago

What are the good alternatives? You can either copy memory manually, or have a system like C++'s that causes lots of tiny allocations, which is very inefficient. Both in space and time. You can also opt for immutable strings to be able to share strings more conveniently and efficiently, at the cost of O(n^2) build time (where automatic optimizations fail) or decreased convenience (explicit string-stream aka string builder).

If efficiency is not a concern for you and you rather like the convenience, good for you.

Personally I feel plain const char pointers (plus length, depending on expected size) are good enough to emulate the immutable string type. For modifiable strings, I still prefer viewing them as chunks of memory, using char pointer + length + capacity. Nice thing is, you can easily go from the former to the latter in the lifetime of any one string. You can also pool many strings in large chunks if you build only one string at any one time.

adrianN2• 8 years ago

Whatever alternative reliably prevents buffer overflows, so pretty much anything but char*.

pjmlp8• 8 years ago

You care about that after a profiler proves that it is actually a problem to the expected delivery SLAs for the application.

Micro-optimizing each line of C code based on gut feeling is not an option if quality matters.

AdmiralAsshat• 8 years ago

In principle, I feel like "Ignore the language-provided standard libraries and use this little macro I wrote in all your code" is bad advice.

If the standard libs are that bad, the community should really lobby for the inclusion of a "safe" string copy into the C standard. Failing that, use a battle-tested third-party library like bstrlib[0].

[0]http://bstring.sourceforge.net/

jstimpfle7• 8 years ago

Strings are so tiny an abstraction over plain memory that every C programmer ends up doing them their own way. And that can be a good thing. I already proposed a couple of ways to represent or muck with strings in my other comment.

cliffy• 8 years ago

How is that a good thing? There are ways of abstracting/handling strings that are strictly safer than other ways. In an unsafe language like C, you're inviting problems by trying to do things in your own idiosyncratic way.

coldtea2• 8 years ago

In what way could ever that be a "good thing" over having 2-3 implementations of strings in the standard library that cover 99% of the different ways and trade-offs, and which each has a method to convert a string to any of the others?

mikeash• 8 years ago

What choice do you have? “Fix the standard” is ideal, but few programmers have the luxury of waiting a decade for the fix to happen and become available in all their target compilers.

pcwalton• 8 years ago

The C standards committee is incredibly conservative, only adding new features if absolutely necessary. Unless the composition and goals of the committee drastically change, a new standard string API will never happen.

If you must write C, you should use a real string library, and not string.h.

iforgotpassword• 8 years ago

Nonsense, strncpy and friends are just fi2•∟1F↔Ñ

astrodust• 8 years ago

They should have a superset of the C standard for writing "secure C", as is important in some kernel or application code.

lmm• 8 years ago

Unfortunately C doesn't have a widely accepted dependency/build manager either.

bitwize2• 8 years ago

strcpy_s, anyone?

brucedawson8• 8 years ago

strcpy_s is a good option for those cases where the appropriate response to having too long an input string is terminating the program. However it is a poor choice when continuing execution is desired.

rurban3• 8 years ago

It is even C11: https://github.com/rurban/safeclib/blob/master/src/str/strcp...

brucedawson8• 8 years ago

strcpy_safe is a template function, not a macro. That immediately makes it orders of magnitude better than you were claiming.

I agree that a standard fix for this would be great. It's been five years since I wrote the post, the solution has been available for many years longer than that, and the standards committee has shown no interest in fixing this issue. In the interim I think it would be irresponsible for developers to continue using strncpy or any of the other unsafe options when strcpy_safe is so simple and effective.

rurban3• 8 years ago

Rather use safeclib. It is C11. It has a safe version of the wrongly spec'ed truncating functions, in this case strncpy_s. This version guarantees NULL termination.

https://rurban.github.io/safeclib/

ahoka• 8 years ago

strncp is fine, however it was never meant for null-terminated strings! A safe alternative is using snprintf and checking the return value for truncation.

kevindqc2• 8 years ago

https://lwn.net/Articles/225108/

brucedawson8• 8 years ago

snprintf is hardly safe. It does guarantee null termination, so that is progress, but it requires the caller to pass the buffer size. As I mention in the "More typing means more errors" section of the article software developers are endlessly creative about ways to pass the wrong buffer size. They usually pass the right buffer size, but 99.9% correct still means a lot of bugs. These bugs can be avoided by using a template function to infer the destination buffer size. A template wrapper around snprintf would work very nicely.

bigpicture• 8 years ago

These bugs can be avoided by using a template function to infer the destination buffer size

I'm a big fan of using strategies to avoid bugs. Another strategy than can be a big help is to spend a little time thinking about what kinds of strings you are going to be using and how big they might be. And then sanitizing your program input based on those sizes. Find the too-long strings before they get into your program and before you start copying them around.

The system I'm responsible for came into being as a set of programs on a Univac II and was a bunch of C and FORTRAN when I joined. Because of the need to pass data between C and FORTRAN programs/code, we had a set of 10-20 standard string sizes, based on what kind of text you had. We no longer write new code in either C or FORTRAN, but maintain the concept of standard sizes for our text. By specifying the kind of text (we also do this with numbers) in the design steps, we save far more time and effort than we spend over the entire SDLC.

hzhou3214• 8 years ago

If we preset the end of buffer with '\0' and then assume the buffer having 1-less capacity, wouldn't it address the strncpy issue?

needlepont3• 8 years ago

Of course, but everyone wants to overdo the complexity of the time worn solution. OMG you need to null terminate the string after all the _other_ gymnastics..gee C sure does suck! Why don't we use rust|go|c++ ad-nauseam. bzero(buf,sz); /memset nazis here/ strncpy(buf,src,sz - 1);

bitwize2• 8 years ago

It doesn't make sense to have to null-terminate the string by hand (maybe) after doing a "safe" STRING copy. Which means it's easy to forget to do and that's a dangerous wart in the design.

Really, stop using C. To quote Hayao Miyazaki, C was a mistake.

hzhou3214• 8 years ago

What I meant is more like:

    #define BUFSIZE 100
    char buf[BUFSIZE+1]
    buf[BUFSIZE]=0 // for global var, it is already zeroed, right?

Then one can call strncpy(buf, BUFSIZE, ...) without worrying about null-termination.

sevensor4• 8 years ago

Well, again, you have to assume that the caller called you right, i.e. that the pointer you were passed points to a buffer with exactly one zero in it, at the end. Furthermore, unless there's also a (potentially wrong) length argument, you have to read every byte of the destination buffer while you're copying and stop when you see a zero. There's a lot to be said for a reimplementation like bstrlib that wraps char arrays in structs and gives you replacements for most of the string functions, but then you still have all these C APIs to deal with that use char*. And you have the overhead of trying to do things right at runtime. Sibling comment mocks languages that do compile-time safety checking, but I totally see the appeal.

hzhou3214• 8 years ago

I am not sure I follow the extent of the problem.

With `strncpy`, the caller assumes the responsibility of buffer size (by specify that size explicitly every single time. I thought that responsibility is not being debated within the current context.

hedora• 8 years ago

I don’t understand why silently truncating strings is considered to be acceptable practice.

If you have the length of both, simply assert the length of the destination is <= the length of the source. Problem solved (bugs become obvious in testing).

brucedawson8• 8 years ago

What about user input, strings coming from files, and in general the diversity of things that can go wrong when your product ships. This is particularly important if your product is attacked by those who would like to exploit it.

Asserts are necessary, but not sufficient.

russdill2• 8 years ago

Because the API is stupid and not intuitive. It's for memory buffers where the string is either NULL terminated or it is the length of the memory buffer. This saves an extra byte. yay?

russdill2• 8 years ago

I'll add that it's really handy for that specific situation as it also avoids leaking uninitialized bytes.

gweinberg• 8 years ago

The more I think about this, the more sense it makes to say you should check the length of the buffer before they copy. I'm not sure an assert is always right, exactly what should be done if the string doesn't fit may depend on what the program is doing. But silently truncating is generally not the right thing to do.

brucedawson8• 8 years ago

strcpy_s does this. It has a template variant to infer the length of the destination buffer and it will terminate the program if the source string is too large.

If the destination buffer doesn't have a fixed length then it will fail to compile.

This is similar to your suggestion with these improvements: - Template deduction of the destination size is done - this is essential to avoid bugs - Detection of overwrites is done in release builds as well as during testing - Detection of overwrites is automatic - it doesn't require manually adding asserts to call sites - strcpy_s exists in some standard libraries

mcguire• 8 years ago

Been a while since I used C++...

How does this work if the size of the destination isn't known at compile time? I.e., it's not "buffer[5]”?

yarosv• 8 years ago

The author states

The focus of this post is on fixed-size string buffers, but the technique applies to any type of fixed-length buffer.

brucedawson8• 8 years ago

You get a compile error.

And this is progress. I have fixed code that looked something like this:

    strncpy(dst, sizeof(dst), src);
    data[sizeof(dst)-1] = 0;

See the bug? Of course not, because it's not visible, but I've seen this pattern used where dst is a pointer. This means that four or eight bytes (three or seven if you don't count the null terminator) gets copied, regardless of how many bytes dst points to. With strcpy_safe an attempt to copy to dst would not work and the developer would have to fix their code.

Every mistake that can be made, will be made.

wmil• 8 years ago

I feel like all of these articles can be summed up with:

"C strings and string functions aren't designed to be safe. If you want safe strings use a library."

kodis• 8 years ago

I'd say "Stop using C and C++ already" or "Stop using null terminated strings already", but I recognize that those are not always practical solutions.

I do have some maintainability concerns though about the promulgation of numerous competing home-brew solutions to a near trivial problem throughout a code base.

Stop using strncpy already (2013)

Related Stories