On Fri, Nov 10, 2023 at 12:18:56 +0100, Alejandro Colomar wrote: > Hi Oskari, > > On Fri, Nov 10, 2023 at 01:06:44AM -0600, Oskari Pirhonen wrote: > > On Thu, Nov 09, 2023 at 13:23:14 +0100, Alejandro Colomar wrote: > > > Don't worry. strncpy(3) won't be deprecated, thanks to tar(1). ;) > > > > > > > Just please don't tar and feather [1] the people who use it ;) > > Hmmm, it just caught me after a year fixing broken strncpy(3) calls. I > was a bit unfair. I'm sorry if I wasn't so nice. Hopefully, we've all > learnt something about string-copying functions. :) > Indeed we have. This whole thread became much more informative than I could've anticipated. And we also got a better wording for strncpy(3) too :) > > > We could maybe add a list of ways people have tried to be clever with > > > strncpy(3) in the past and failed, and then explain why those uses are > > > broken. This could be in a BUGS section. > > > > > > > This would be a very fun read. > > I'll write it then! :D > > > > > ... snip ... > > > > > > > Also, I've seen a lot of off-by-one bugs in calls to strncpy(3), so no, > > > > > it's not correct code. It's rather dangerous code that just happens to > > > > > not be vulnerable most of the time. > > > > > > > > So will all the custom strlen(3)+memcpy(3)-based replacements suddenly be > > > > immune to off-by-one bugs? > > > > > > Slightly. Here's the typical use of strlen(3)+strcpy(3): > > > > > > if (strlen(src) >= dsize) > > > goto error; > > > strcpy(dst, src); > > > > > > There's no +1 or -1 in that code, so it's hard to make an off-by-one > > > mistake. Okay, you may have seen that it has a '>=', which one could > > > accidentally replace by a '>', causing an off-by-one. I'd wrap that > > > thing in a strxcpy() wrapper so you avoid repetition. > > > > > > > Might I go so far as to recommend strnlen(3) instead of strlen(3)? That > > way, instead of blindly looking for a null terminator, you stop after a > > predetermined max length. Especially nice for untrusted input where you > > can't make assumptions on the "fitness for a purpose" of what's being > > fed in. > > > > if (src == NULL || strnlen(src, dsize) == dsize) > > goto error; > > strcpy(dst, src); > > A NULL check shouldn't be necessary (no other copying functions have, > and that's not a big deal with them, although I have mixed feelings > about things like memcpy(dst, NULL, 0)). > > About strnlen(3), you're right, and Paul also pointed that out. See the > other mail I sent to the list with an inline implementation of strxcpy() > using strnlen(3). > Yep. I saw it just before replying to this message. > > > > This, of course, assumes you have POSIX at your disposal. > > I always assume this. If not, please ask your vendor to provide a POSIX > layer. Or at least the parts of POSIX that can be implemented in a > free-standing implementation. Or stop using that vendor. > > > > > I'm writing this before going to bed. I did briefly sanity check it with > > a simple test prog, but it would be quite ironic if I missed something > > wouldn't it... > > Looks good at first glance. :) > Dev 1: It passes all tests. Dev 2: Ship it. Users: *proceed to break it anyway* - Oskari
Attachment:
signature.asc
Description: PGP signature