strlcpy [LWN.net]

From:		Theo de Raadt <deraadt@cvs.openbsd.org>
To:		lwn@lwn.net
Subject:		strlcpy
Date:		Thu, 05 Jun 2003 14:11:00 -0600

Two thing you guys might like to be aware of:

1) We certainly hope that the Linux version is 100% compatible. Even
a teeny little incompatibility here would be utterly dumb. Ours is
under an ISC-like license -- you cannot get more free than that, and
we would be utterly pissed off if Linus went and made incompatible
changes (and I think the community should also be; note that Linux
snprintf is not 100% complaint either). Solaris had a bug in the
first strlcpy/strlcat they shipped, and I believe they have now
adjusted to just using our code -- to avoid incompatibilities.

2) Our bootblocks, kernels, and all of userland (except for a few GNU
things like gcc, cvs, and binutils) are not using strcpy, strcat,
sprintf, or vsprintf anymore. Essentially our entire tree has been
converted to use the safer (bounded) variants.

to post comments

strlcpy

Posted Jun 12, 2003 3:51 UTC (Thu) by JoeBuck (subscriber, #2330) [Link] (1 responses)

If you don't have a document anywhere, explaining these relatively small functions clearly enough so that any competent programmer can produce a correct implementation, there's a problem. "Just use our code" is OK, but doesn't really suffice for good security analysis unless you want everyone to do duplicate work.

Also, many common uses of strcpy are completely safe and more efficient than strlcpy. Removing every use is just superstition.

strlcpy

Posted Jun 12, 2003 7:46 UTC (Thu) by set (guest, #4788) [Link]

Linus claims to have maintained *BSD compatibility, though he
wrote the functions himself:

"Ok, I did my own versions, since (a) I had already started and your
patches wouldn't apply, and (b) I hate adding a zillion lines of extra
copyright notices for a 5-line function..."

(strlcat was added as well)

I dont think strcpy is the main target, as much as strncpy. Linus again:

"Yeah, "strncpy()" is a frigging disaster when it comes to '\0', in many
ways. We should probably disallow using strncpy(), and aim for a _sane_
implementation that does what we actually want (none of that zero-padding
crap, and _always_ put a NUL at the end). I bet that is what most current
strncpy() users actually would want."

A light grep of 2.5.70 shows many uses of strlcpy, strncpy, and strcpy
out there...

Paul

strlcpy

Posted Jun 12, 2003 4:50 UTC (Thu) by ncm (guest, #165) [Link] (7 responses)

Worse, each instance of strlcpy in a program indicates a potential bug in its own right. strlcpy has been rejected by Posix, I am told, at least twice, for that reason. While less likely to participate in a buffer overflow, it is likely to introduce more subtle bugs.

First, since it performs a partial copy, it's easy for code (even code that checks and acts correctly on its return value) to end up using the partial, incorrect copy.

Second, if told it has a zero-length buffer, it doesn't even null-terminate it, leaving whatever was already present in the buffer in place.

A much more useful definition, if there must be something like strlcpy, would be to call abort() if the argument string doesn't fit in the buffer. (If called from within the Linux kernel, an Oops would not be out of place.) The only circumstance I can think of where a partial copy wouldn't always be worse than entirely failing is when copying an error message into a fixed-size display field. That case seems to me rare enough not to merit such a complicated and dangerous function.

If strlcpy is part of the kernel's library, I can only hope that any patch that relies on it is rejected as too poorly coded. A set of "canary" utility functions used to detect bad code would not be a bad addition to the kernel, but their purpose would be hard to keep secret.

strlcpy

Posted Jun 12, 2003 7:12 UTC (Thu) by bgilbert (subscriber, #4738) [Link] (1 responses)

> Second, if told it has a zero-length buffer, it doesn't even
> null-terminate it, leaving whatever was already present in the
> buffer in place.

If told it has a zero-length buffer, where is it supposed to put the null? Yes, the zero length probably indicates a bug of some sort, but you seem to be proposing that the function should deliberately write past the end of the buffer in this case. That seems rather counterproductive.

strlcpy

Posted Jun 13, 2003 0:14 UTC (Fri) by ncm (guest, #165) [Link]

"you seem to be proposing that the function should deliberately write past the end of the buffer in this case"

I see that you stopped reading at that point. Pray read on. Programming errors shouldn't silently be worked around, they should result in a lot of noise, smoke, and flame.

strlcpy

Posted Jun 12, 2003 13:31 UTC (Thu) by mrshiny (guest, #4266) [Link]

You are right; strlcpy can mask certain coding errors. But programmers always make errors; what's worse: a coding error that results in incorrect (but non-memory-corrupting) behaviour, or a coding error that results in a buffer overflow (which is usually guaranteed to be a sure path to denial of service or code injection)?

I'd rather have a programmer's mistakes masked as incorrect behaviour that won't allow attackers to take over my computer, thank you.

strlcpy

Posted Jun 12, 2003 15:10 UTC (Thu) by Peter (guest, #1127) [Link] (1 responses)

Worse, each instance of strlcpy in a program indicates a potential bug in its own right.

That's like saying "each instance of malloc in a program indicates a potential memory leak in its own right." True, but uninteresting.

strlcpy truncates your string, sure - but it gives you enough information to trivially detect this, so you can either

use the truncated string,
return some sort of error condition, or
reallocate your buffer and try again.

I've seen many instances in code where a buffer is allocated to be plenty long enough for any sane input. In which case if the program truncates your input and thus behaves incorrectly, the user has only himself to blame, for giving "unreasonable" input. For example, if a web server returns a "404 not found" because you asked for a filename longer than (say) 1024 characters - I would consider that, not a bug, but just a "that case is not worth the trouble to support, and `404 Not Found' is a perfectly reasonable approximation for `You Seem To Be Trying To Overflow A Buffer, Get Lost'."

As far as security implications go: all user input should in general be considered untrusted and invalid until proven otherwise. The fact that it may or may not have been truncated is irrelevent.

Obviously there are other cases where it is appropriate to detect the truncation and deal with it some way (either with an explicit error condition, or a realloc). But it would seem counterproductive to deny the existence of cases where truncation really is a safe and correct option.

strlcpy

Posted Jun 12, 2003 16:02 UTC (Thu) by remijnj (guest, #5838) [Link]

I completely agree with this but the example is bad because the webserver SHOULD return "414 Request-URI Too Long". See RFC2068 (HTTP/1.1) about this.

Joost

strlcpy

Posted Jun 12, 2003 15:55 UTC (Thu) by tjc (guest, #137) [Link]

First, since it performs a partial copy, it's easy for code (even code that checks and acts correctly on its return value) to end up using the partial, incorrect copy.

But this is better behavior than strncpy(), which also performs a partial copy but leaves it unterminated.

I don't see that it's the job of low-level library functions to anticipate every error that a programmer might possibly make.

strlcpy

Posted Jun 14, 2003 1:12 UTC (Sat) by giraffedata (guest, #1954) [Link]

All the criticisms of strlcpy based on the idea that it's always wrong to copy just part of a string ignore the fact that there are millions of places in code where it's no big deal to copy just part of the string occasionally, and those are the places where the programmer doesn't want to waste precious time dealing with the possibility that his buffer is too small.

For example, I'm going to write the message on the screen for a human user to read. So what if the message is truncated to 1000 characters? Let me just strlcpy it and be done with it.