Re: strncpy clarify result may not be null terminated

[Alejandro Colomar <alx@kernel.org>]

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

On Wed, Nov 08, 2023 at 08:33:34PM +0100, Alejandro Colomar wrote:
> Hi Matthew,
> 
> On Tue, Nov 07, 2023 at 09:12:37PM -0500, Matthew House wrote:
> > On Tue, Nov 7, 2023 at 8:21 AM Alejandro Colomar <alx@kernel.org> wrote:
> > > On Tue, Nov 07, 2023 at 11:52:44AM +0000, Jonny Grant wrote:
> > > > We see things differently, I'm on the C standard side on this one. Would any information change your mind?
> > >
> > > It's difficult to say, but I doubt it.  But let me ask you something:
> > > In what cases would you find strncpy(3) appropriate to use, and why?
> > > Maybe if I understand that it helps.
> > >
> > > Kind regards,
> > > Alex
> > 
> > Man pages aren't read only by people writing new code, but also by people
> > reading and modifying existing code. And despite your preferences regarding
> > which functions ought to be used to produce strings, it's a widespread (and
> > correct) practice to produce a string from the character sequence created
> > by strncpy(3). There are two ways of doing this, either by setting the last
> > character of the destination buffer to null if you want to produce a
> > truncated string, or by testing the last character against zero if you want
> > to detect truncation and raise an error.
> 
> It is not strncpy(3) who truncated, but the programmer by adding a NULL

Oops.  s/NULL/NUL/

> in buff[BUFSIZ - 1].  In the following snippet, strncpy(3) will not
> truncate:
> 
> 	char cs[3];
> 
> 	strncpy(cs, "foo", 3);
> 
> And yet your code doing if (cs[2] != '\0') { goto error; } would think
> it did.  That's because you deformed strncpy(3) to implement a poor
> man's strlcpy(3).  
> 
> 
> 	char cs[3];
> 
> 	strncpy(cs, "foo", 3);
> 	cs[2] = '\0';  // The truncation is here, not in strncpy(3).
> 
> > I'm not aware of any alternative to a strncpy(3)-based snippet for
> > producing a possibly-truncated copy of a string, except for your preferred
> > strlcpy(3) or stpecpy(3), which aren't available to anyone without a
> 
> The Linux kernel has strscpy(3), which is also good, but is not
> available to user space.
> 
> > brand-new glibc (nor, by extension, any applications or libraries that want
> 
> libbsd has provided strlcpy(3) since basically forever.  It is a very
> portable library.  You don't need a brand-new glibc for having
> strlcpy(3).
> 
> <https://libbsd.freedesktop.org/wiki/>
> 
> > to support people without a brand-new glibc, nor any libraries that want to
> > support other platforms like Windows with only ISO C and POSIX-ish
> 
> If you program for Windows, it depends.  If you have POSIX available,
> you may be able to port libbsd; I don't know.  In any case, I don't
> case about Windows enough.  You could always write your own string-
> copying function for Windows.
> 
> > functions); snprintf(3), which has the insidious flaw of not supporting
> > more than INT_MAX characters on pain of UB, and also produces a warning if
> > the compiler notices the possible truncation; or strlen(3) + min() +
> > memcpy(3) + manually adding a null terminator, which is certainly more
> > explicit in its intent, and avoids strncpy(3)'s zero-filling behavior if
> > that poses a performance problem, but similarly opens up room for
> > off-by-one errors.
> 
> More than the performance problem, I'm more worried about the
> maintainability of strncpy(3).  When 20 years from now, a programmer
> reading a piece of code full of strncpy(3) wants to migrate to a sane
> function like strlcpy(3) or strcpy(3), the programmer needs to
> understand if the zeroing was purposeful or just accidental.  Because
> by using strlcpy(3), it may start leaking some trailing data if the
> trailing of the buffer is meaningful to some program.
> 
> > 
> > For the sake of reference, I looked into a few big C and C++ projects to
> > see how often a strncpy(3)-based snippet was used to produce a truncated
> > copy. I found 18 instances in glibc 2.38, 2 in util-linux 2.39.2 (in spite
> > of its custom xstrncpy() function), 61 in GNU binutils 2.41, 43 in
> > GDB 13.2, 1 in LLVM 17.0.4, 7 in CPython 3.12.0, 99 in OpenJDK 22+22,
> > 10 in .NET Runtime 7.0.13, 3 in V8 12.1.82, and 86 in Firefox 120.0. (Note
> > that I haven't filtered out vendored dependencies, so there's a little bit
> > of double-counting.) It seems like most codebases that don't ban strncpy(3)
> > use a derived snippet somewhere or another. Also, I found 3 instances in
> > glibc 2.38 and 5 instances in Firefox 120.0 of detecting truncation by
> > checking the last character.
> 
> I know.  I've been rewriting the code handling strings in shadow-utils
> for the last year, and ther was a lot of it.  I fixed several small bugs
> in the process, so I recommend avoiding it.
> 
> > 
> > So these two snippets really are widespread, especially among the long tail
> > of smaller C and C++ applications and libraries that don't perform enough
> > string manipulation that it warrants creating a custom set of more-
> > foolproof wrapper functions (at least, in the opinion of their authors).
> 
> 
> 
> > Thus, since they're not going away, it would be useful for anyone reading
> > the code to understand the concept behind how these two snippets work, that
> > the only difference between the strncpy(3)'s special "character sequence"
> > and an ordinary C string is an additional null terminator at the end of the
> > destination buffer.
> 
> This is part of string_copying(7):
> 
> DESCRIPTION
>    Terms (and abbreviations)
>      string (str)
>             is  a  sequence  of zero or more non‐null characters followed by a
>             null byte.
> 
>      character sequence
>             is a sequence of zero or  more  non‐null  characters.   A  program
>             should  never use a character sequence where a string is required.
>             However, with appropriate care, a string can be used in the  place
>             of a character sequence.
> 
> I think that is very explicit in the difference.  strncpy(3) refers to
> that page for understanding the differences, so I think it is
> documented.
> 
> strncpy(3):
> CAVEATS
>      The  name  of  these  functions  is confusing.  These functions produce a
>      null‐padded character sequence, not a string (see string_copying(7)).
> 
> > 
> > In other words, strncpy(3) doesn't create a truncated string, but it
> > creates something which can be easily turned into to a truncated string,
> > and that's its most relevant quality for most of its uses in existing code.
> > Further, apart from snprintf(3), there's no other portable way to produce a
> > truncated string without manual arithmetic. Thus, I'd also find it
> 
> Portable is relative.  With libbsd, you can port to most POSIX systems.
> Windows is another story.
> 
> > reasonable to highlight precisely why strncpy(3)'s output isn't a string
> 
> How about this?:
> 
> diff --git a/man3/stpncpy.3 b/man3/stpncpy.3
> index d4c2ce83d..c80c8b640 100644
> --- a/man3/stpncpy.3
> +++ b/man3/stpncpy.3
> @@ -108,7 +108,10 @@ .SH HISTORY
>  .SH CAVEATS
>  The name of these functions is confusing.
>  These functions produce a null-padded character sequence,
> -not a string (see
> +not a string.
> +While strings have a terminating NUL byte,
> +character sequences do not have any terminating byte
> +(see
>  .BR string_copying (7)).
>  .P
>  It's impossible to distinguish truncation by the result of the call,
> 
> 
> > (viz., the lack of a null terminator), instead of trying to insist that its
> > output is worlds apart from anything string-related, especially given the
> > volume of existing correct code that belies that notion.
> 
> It is not correct code.  That code is doing extra work which confuses
> maintainers.  It is a lot like writing dead code, since you're writing
> zeros that nobody is reading, which confuses maintainers.
> 
> 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.
> 
> > 
> > Or, to answer your question, "It's appropriate to keep using strncpy(3) in
> > existing code where it's currently used as part of creating a truncated
> > string, and it's not especially inappropriate to use strncpy(3) in new code
> > as part of creating a truncated string, if the code must support platforms
> > without strlcpy(3) or similar, and if the resulting snippets are few enough
> > and well-commented enough that they create less mental load than creating
> > and maintaining a custom helper function."
> 
> strncpy(3) calls are never well documented.  Do you add a comment in
> each such call saying "this zeroing is superfluous"?  Probably not.
> 
> > 
> > (As an aside, I find the remark in the man page that "It's impossible to
> > distinguish truncation by the result of the call" extremely misleading at
> > best, since truncation can easily be distinguished by inspecting the last
> > output character.)
> 
> Again, strncpy(3)'s truncation is impossible to detect.  What you can
> detect is that your construct that resembles strlcpy(3) truncates, which
> is a different thing.
> 
> Thanks,
> Alex
> 
> > 
> > Thank you,
> > Matthew House
> 
> -- 
> <https://www.alejandro-colomar.es/>



-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]