Re: strncpy clarify result may not be null terminated

[Oskari Pirhonen <xxc3ncoredxx@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3350 bytes --]

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]