Re: [PATCH] strcat.3: SYNOPSIS: Fix the size of 'dest'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Russ!

On 12/5/22 18:48, Russ Allbery wrote:
Alejandro Colomar via Libc-alpha <libc-alpha@xxxxxxxxxxxxxx> writes:

I'm continuing my indiscriminated shooting against broken functions.
I don't remember if I ever used it, but it got me surprised for how much
broken it is.

Please kill this function in glibc.  The updated prototype using a bit
of imagination to overextend VLA syntax to show how it behaves, shows
how broken it is.

It is impossible to use this function correctly (okay, it you try hard,
you can, but only for the pleasure of using it without crashing, not for
anything useful).

Hi Alejandro,

I'm just a bystander here, but I've seen a few proposals like this go by,
and I'm not sure anyone has yet said explicitly that these functions are
incredibly widely used in real C code out in the world.

Yeah, I'd say sadly.

 They predate all
of the functions that you are recommending as replacements for them, so
they're common in old, portable C.

Well, the building blocks for implementing any of the less-portable functions are already portable: memcpy(3), memccpy(3), strlen(3), ...


I think the work you're doing to document that they should never be used
for new code in the man pages is great; please continue!

Thanks!  Will do :)

 Although it may
be useful to carry with that some caveats about portability; some of the
replacements you've mentioned are going to pose portability challenges.

libbsd already does a good job of making them portable:

<https://libbsd.freedesktop.org/wiki/>

It's available in virtually all Linux distributions, and then there are ports to other systems such as Solaris. I guess if some system is not yet supported by it, one could port it.


I'm a bit more dubious about glibc marking these functions as formally
deprecated unless the C standard also deprecates them (as happened with
gets, which is probably the best precedent for what you're trying to
accomplish).

Someone has to propose its cold death. Otherwise, we may live with it for decades, if not centuries, and people will continue to comit crimes in their names. :)


For the record, there is quite a lot of code out there that uses strcpy
and strcat safely,

Oh, I didn't (yet?) kill those. I don't recommend them personally, but gcc fortifications have improved so much that it's harder to commit crimes with them. There are other ways to get the same simplicity as with those, and even better performance, but they are certainly still useful in some cases.

The ones I killed so far are strNcpy(3) and strNcat(3), which simply can't be used safely.

using a pattern where first the total length of the
resulting string is calculated, that much space is allocated, and then it
is assembled with strcpy and strcat.  This used to be the standard way to
assemble strings in C before strlcpy or asprintf existed (and neither of
those functions are all that portable even now; I would still hesittate to
use either without a configure probe and a replacement implementation).

If you use autotools, you can follow what we did in shadow for getting readpassphrase(3bsd) from libbsd:

<https://github.com/shadow-maint/shadow/pull/573>

 I
think you're exaggerating the difficulty of using them safely a tiny bit,
but maybe that's just because I'm an old C programmer for whom that
pattern is a finger macro.

No, I might be exaggerating a little bit if I said that about strcpy(3)/strcat(3). But for strNcpy(3)/strNcat(3), I stand by my point: they are never the function you should use.


While you can use strncpy(3) or strncat(3) without breaking your program, they are so wrongly designed that it's really hard to not break it. There's _always_ a simpler _and_ safer function for the job. Also, their names don't document the intention of the programmer.

I prefer actual macros (or functions) rather than finger macros. Finger macros, especially multi-line patterns, tend to hide bugs. If there's any value in strncpy(3) (there isn't), hide it in a real macro and use the macro. But if you do that, you can just use memcpy(3) for the implementation as well.


It's also probably worth saying explicitly that strlcpy and strlcat are
still quite dangerous functions and need to be used very carefully.
 They
do solve the problem of buffer overflows when used properly, but they
replace that problem with silent truncation, which can also cause security
vulnerabilities in the right circumstances. >


Yes, truncation is a problem of strlcpy(3) (or actually, of any safe string copy functions). But good string copy functions will at least make it easy for you to check for it. strNcpy(3) and strNcat(3) actually don't even allow you to check for it, which is yet another reason to avoid them.

At least with strlcpy(3) you can find if you're checking for truncation by grepping it. If you find '^ *strlcpy(', it's likely that you forgot to check.

Cheers,

Alex


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

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


[Index of Archives]     [Kernel Documentation]     [Netdev]     [Linux Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux