* [-Wstringop-overflow=] strncat(3)
@ 2022-12-14 22:45 Alejandro Colomar
2022-12-14 22:51 ` Alejandro Colomar
2022-12-14 22:57 ` Andrew Pinski
0 siblings, 2 replies; 7+ messages in thread
From: Alejandro Colomar @ 2022-12-14 22:45 UTC (permalink / raw)
To: gcc; +Cc: linux-man, GNU C Library
[-- Attachment #1.1: Type: text/plain, Size: 3516 bytes --]
Hi,
I was rewriting the strncat(3) manual page, and when I tried to compile the
example program, I got a surprise from the compiler.
Here goes the page:
strncat(3) Library Functions Manual strncat(3)
NAME
strncat - concatenate a null‐padded character sequence into a
string
LIBRARY
Standard C library (libc, -lc)
SYNOPSIS
#include <string.h>
char *strncat(char *restrict dst, const char src[restrict .sz],
size_t sz);
DESCRIPTION
This function catenates the input character sequence contained in
a null‐padded fixed‐width buffer, into a string at the buffer
pointed to by dst. The programmer is responsible for allocating a
buffer large enough, that is, strlen(dst) + strnlen(src, sz) + 1.
An implementation of this function might be:
char *
strncat(char *restrict dst, const char *restrict src, size_t sz)
{
int len;
char *end;
len = strnlen(src, sz);
end = dst + strlen(dst);
end = mempcpy(end, src, len);
*end = '\0';
return dst;
}
RETURN VALUE
strncat() returns dest.
ATTRIBUTES
[...]
STANDARDS
POSIX.1‐2001, POSIX.1‐2008, C89, C99, SVr4, 4.3BSD.
CAVEATS
The name of this function is confusing. This function has no re‐
lation with strncpy(3).
If the destination buffer is not large enough, the behavior is un‐
defined. See _FORTIFY_SOURCE in feature_test_macros(7).
BUGS
This function can be very inefficient. Read about Shlemiel
the painter ⟨https://www.joelonsoftware.com/2001/12/11/
back-to-basics/⟩.
EXAMPLES
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int
main(void)
{
char buf[BUFSIZ];
size_t len;
buf[0] = '\0'; // There’s no ’cpy’ function to this ’cat’.
strncat(buf, "Hello ", 6);
strncat(buf, "world", 42); // Padding null bytes ignored.
strncat(buf, "!", 1);
len = strlen(buf);
printf("[len = %zu]: <%s>\n", len, buf);
exit(EXIT_SUCCESS);
}
SEE ALSO
string(3), string_copy(3)
Linux man‐pages (unreleased) (date) strncat(3)
And when you compile that, you get:
$ cc -Wall -Wextra ./strncat.c
./strncat.c: In function ‘main’:
./strncat.c:12:12: warning: ‘strncat’ specified bound 6 equals source length
[-Wstringop-overflow=]
12 | strncat(buf, "Hello ", 6);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
./strncat.c:14:12: warning: ‘strncat’ specified bound 1 equals source length
[-Wstringop-overflow=]
14 | strncat(buf, "!", 1);
| ^~~~~~~~~~~~~~~~~~~~
So, what? Where's the problem? This function does exactly that: "take an
unterminated character sequence and catenate it to an existing string". Clang
seems to be fine with the code.
Cheers,
Alex
--
<http://www.alejandro-colomar.es/>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [-Wstringop-overflow=] strncat(3)
2022-12-14 22:45 [-Wstringop-overflow=] strncat(3) Alejandro Colomar
@ 2022-12-14 22:51 ` Alejandro Colomar
2022-12-14 22:51 ` Alejandro Colomar
2022-12-14 22:57 ` Andrew Pinski
1 sibling, 1 reply; 7+ messages in thread
From: Alejandro Colomar @ 2022-12-14 22:51 UTC (permalink / raw)
To: gcc; +Cc: linux-man, GNU C Library
[-- Attachment #1.1: Type: text/plain, Size: 4583 bytes --]
On 12/14/22 23:45, Alejandro Colomar wrote:
> Hi,
>
> I was rewriting the strncat(3) manual page, and when I tried to compile the
> example program, I got a surprise from the compiler.
>
> Here goes the page:
>
>
> strncat(3) Library Functions Manual strncat(3)
>
> NAME
> strncat - concatenate a null‐padded character sequence into a
> string
>
> LIBRARY
> Standard C library (libc, -lc)
>
> SYNOPSIS
> #include <string.h>
>
> char *strncat(char *restrict dst, const char src[restrict .sz],
> size_t sz);
>
> DESCRIPTION
> This function catenates the input character sequence contained in
> a null‐padded fixed‐width buffer, into a string at the buffer
> pointed to by dst. The programmer is responsible for allocating a
> buffer large enough, that is, strlen(dst) + strnlen(src, sz) + 1.
>
> An implementation of this function might be:
>
> char *
> strncat(char *restrict dst, const char *restrict src, size_t sz)
> {
> int len;
> char *end;
>
> len = strnlen(src, sz);
> end = dst + strlen(dst);
> end = mempcpy(end, src, len);
> *end = '\0';
>
> return dst;
> }
>
> RETURN VALUE
> strncat() returns dest.
>
> ATTRIBUTES
> [...]
>
> STANDARDS
> POSIX.1‐2001, POSIX.1‐2008, C89, C99, SVr4, 4.3BSD.
>
> CAVEATS
> The name of this function is confusing. This function has no re‐
> lation with strncpy(3).
>
> If the destination buffer is not large enough, the behavior is un‐
> defined. See _FORTIFY_SOURCE in feature_test_macros(7).
>
> BUGS
> This function can be very inefficient. Read about Shlemiel
> the painter ⟨https://www.joelonsoftware.com/2001/12/11/
> back-to-basics/⟩.
>
> EXAMPLES
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
>
> int
> main(void)
> {
> char buf[BUFSIZ];
> size_t len;
>
> buf[0] = '\0'; // There’s no ’cpy’ function to this ’cat’.
> strncat(buf, "Hello ", 6);
> strncat(buf, "world", 42); // Padding null bytes ignored.
> strncat(buf, "!", 1);
> len = strlen(buf);
> printf("[len = %zu]: <%s>\n", len, buf);
>
> exit(EXIT_SUCCESS);
> }
>
> SEE ALSO
> string(3), string_copy(3)
>
> Linux man‐pages (unreleased) (date) strncat(3)
>
>
> And when you compile that, you get:
>
> $ cc -Wall -Wextra ./strncat.c
> ./strncat.c: In function ‘main’:
> ./strncat.c:12:12: warning: ‘strncat’ specified bound 6 equals source length
> [-Wstringop-overflow=]
> 12 | strncat(buf, "Hello ", 6);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> ./strncat.c:14:12: warning: ‘strncat’ specified bound 1 equals source length
> [-Wstringop-overflow=]
> 14 | strncat(buf, "!", 1);
> | ^~~~~~~~~~~~~~~~~~~~
>
>
> So, what? Where's the problem? This function does exactly that: "take an
> unterminated character sequence and catenate it to an existing string". Clang
> seems to be fine with the code.
Maybe it's saying that I should be using strncat(buf, "!"); because the length
is useless?
>
> Cheers,
>
> Alex
>
>
--
<http://www.alejandro-colomar.es/>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [-Wstringop-overflow=] strncat(3)
2022-12-14 22:51 ` Alejandro Colomar
@ 2022-12-14 22:51 ` Alejandro Colomar
0 siblings, 0 replies; 7+ messages in thread
From: Alejandro Colomar @ 2022-12-14 22:51 UTC (permalink / raw)
To: gcc; +Cc: linux-man, GNU C Library
[-- Attachment #1.1: Type: text/plain, Size: 4839 bytes --]
On 12/14/22 23:51, Alejandro Colomar wrote:
>
>
> On 12/14/22 23:45, Alejandro Colomar wrote:
>> Hi,
>>
>> I was rewriting the strncat(3) manual page, and when I tried to compile the
>> example program, I got a surprise from the compiler.
>>
>> Here goes the page:
>>
>>
>> strncat(3) Library Functions Manual strncat(3)
>>
>> NAME
>> strncat - concatenate a null‐padded character sequence into a
>> string
>>
>> LIBRARY
>> Standard C library (libc, -lc)
>>
>> SYNOPSIS
>> #include <string.h>
>>
>> char *strncat(char *restrict dst, const char src[restrict .sz],
>> size_t sz);
>>
>> DESCRIPTION
>> This function catenates the input character sequence contained in
>> a null‐padded fixed‐width buffer, into a string at the buffer
>> pointed to by dst. The programmer is responsible for allocating a
>> buffer large enough, that is, strlen(dst) + strnlen(src, sz) + 1.
>>
>> An implementation of this function might be:
>>
>> char *
>> strncat(char *restrict dst, const char *restrict src, size_t sz)
>> {
>> int len;
>> char *end;
>>
>> len = strnlen(src, sz);
>> end = dst + strlen(dst);
>> end = mempcpy(end, src, len);
>> *end = '\0';
>>
>> return dst;
>> }
>>
>> RETURN VALUE
>> strncat() returns dest.
>>
>> ATTRIBUTES
>> [...]
>>
>> STANDARDS
>> POSIX.1‐2001, POSIX.1‐2008, C89, C99, SVr4, 4.3BSD.
>>
>> CAVEATS
>> The name of this function is confusing. This function has no re‐
>> lation with strncpy(3).
>>
>> If the destination buffer is not large enough, the behavior is un‐
>> defined. See _FORTIFY_SOURCE in feature_test_macros(7).
>>
>> BUGS
>> This function can be very inefficient. Read about Shlemiel
>> the painter ⟨https://www.joelonsoftware.com/2001/12/11/
>> back-to-basics/⟩.
>>
>> EXAMPLES
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <string.h>
>>
>> int
>> main(void)
>> {
>> char buf[BUFSIZ];
>> size_t len;
>>
>> buf[0] = '\0'; // There’s no ’cpy’ function to this ’cat’.
>> strncat(buf, "Hello ", 6);
>> strncat(buf, "world", 42); // Padding null bytes ignored.
>> strncat(buf, "!", 1);
>> len = strlen(buf);
>> printf("[len = %zu]: <%s>\n", len, buf);
>>
>> exit(EXIT_SUCCESS);
>> }
>>
>> SEE ALSO
>> string(3), string_copy(3)
>>
>> Linux man‐pages (unreleased) (date) strncat(3)
>>
>>
>> And when you compile that, you get:
>>
>> $ cc -Wall -Wextra ./strncat.c
>> ./strncat.c: In function ‘main’:
>> ./strncat.c:12:12: warning: ‘strncat’ specified bound 6 equals source length
>> [-Wstringop-overflow=]
>> 12 | strncat(buf, "Hello ", 6);
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~
>> ./strncat.c:14:12: warning: ‘strncat’ specified bound 1 equals source length
>> [-Wstringop-overflow=]
>> 14 | strncat(buf, "!", 1);
>> | ^~~~~~~~~~~~~~~~~~~~
>>
>>
>> So, what? Where's the problem? This function does exactly that: "take an
>> unterminated character sequence and catenate it to an existing string". Clang
>> seems to be fine with the code.
>
> Maybe it's saying that I should be using strncat(buf, "!"); because the length
oops; of course, I meant strcat().
> is useless?
>
>>
>> Cheers,
>>
>> Alex
>>
>>
>
--
<http://www.alejandro-colomar.es/>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [-Wstringop-overflow=] strncat(3)
2022-12-14 22:45 [-Wstringop-overflow=] strncat(3) Alejandro Colomar
2022-12-14 22:51 ` Alejandro Colomar
@ 2022-12-14 22:57 ` Andrew Pinski
2022-12-14 23:14 ` Alejandro Colomar
1 sibling, 1 reply; 7+ messages in thread
From: Andrew Pinski @ 2022-12-14 22:57 UTC (permalink / raw)
To: Alejandro Colomar; +Cc: gcc, linux-man, GNU C Library
On Wed, Dec 14, 2022 at 2:46 PM Alejandro Colomar via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> Hi,
>
> I was rewriting the strncat(3) manual page, and when I tried to compile the
> example program, I got a surprise from the compiler.
>
> Here goes the page:
>
>
> strncat(3) Library Functions Manual strncat(3)
>
> NAME
> strncat - concatenate a null‐padded character sequence into a
> string
>
> LIBRARY
> Standard C library (libc, -lc)
>
> SYNOPSIS
> #include <string.h>
>
> char *strncat(char *restrict dst, const char src[restrict .sz],
> size_t sz);
>
> DESCRIPTION
> This function catenates the input character sequence contained in
> a null‐padded fixed‐width buffer, into a string at the buffer
> pointed to by dst. The programmer is responsible for allocating a
> buffer large enough, that is, strlen(dst) + strnlen(src, sz) + 1.
>
> An implementation of this function might be:
>
> char *
> strncat(char *restrict dst, const char *restrict src, size_t sz)
> {
> int len;
> char *end;
>
> len = strnlen(src, sz);
> end = dst + strlen(dst);
> end = mempcpy(end, src, len);
> *end = '\0';
>
> return dst;
> }
>
> RETURN VALUE
> strncat() returns dest.
>
> ATTRIBUTES
> [...]
>
> STANDARDS
> POSIX.1‐2001, POSIX.1‐2008, C89, C99, SVr4, 4.3BSD.
>
> CAVEATS
> The name of this function is confusing. This function has no re‐
> lation with strncpy(3).
>
> If the destination buffer is not large enough, the behavior is un‐
> defined. See _FORTIFY_SOURCE in feature_test_macros(7).
>
> BUGS
> This function can be very inefficient. Read about Shlemiel
> the painter ⟨https://www.joelonsoftware.com/2001/12/11/
> back-to-basics/⟩.
>
> EXAMPLES
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
>
> int
> main(void)
> {
> char buf[BUFSIZ];
> size_t len;
>
> buf[0] = '\0'; // There’s no ’cpy’ function to this ’cat’.
> strncat(buf, "Hello ", 6);
> strncat(buf, "world", 42); // Padding null bytes ignored.
> strncat(buf, "!", 1);
> len = strlen(buf);
> printf("[len = %zu]: <%s>\n", len, buf);
>
> exit(EXIT_SUCCESS);
> }
>
> SEE ALSO
> string(3), string_copy(3)
>
> Linux man‐pages (unreleased) (date) strncat(3)
>
>
> And when you compile that, you get:
>
> $ cc -Wall -Wextra ./strncat.c
> ./strncat.c: In function ‘main’:
> ./strncat.c:12:12: warning: ‘strncat’ specified bound 6 equals source length
> [-Wstringop-overflow=]
> 12 | strncat(buf, "Hello ", 6);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> ./strncat.c:14:12: warning: ‘strncat’ specified bound 1 equals source length
> [-Wstringop-overflow=]
> 14 | strncat(buf, "!", 1);
> | ^~~~~~~~~~~~~~~~~~~~
>
>
> So, what? Where's the problem? This function does exactly that: "take an
> unterminated character sequence and catenate it to an existing string". Clang
> seems to be fine with the code.
See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83404 and the
background of why the warning was added here:
https://www.us-cert.gov/bsi/articles/knowledge/coding-practices/strncpy-and-strncat.
Thanks,
Andrew Pinski
>
> Cheers,
>
> Alex
>
>
> --
> <http://www.alejandro-colomar.es/>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [-Wstringop-overflow=] strncat(3)
2022-12-14 22:57 ` Andrew Pinski
@ 2022-12-14 23:14 ` Alejandro Colomar
2022-12-15 20:50 ` Martin Sebor
0 siblings, 1 reply; 7+ messages in thread
From: Alejandro Colomar @ 2022-12-14 23:14 UTC (permalink / raw)
To: Andrew Pinski; +Cc: gcc, linux-man, GNU C Library, groff, Martin Sebor
[-- Attachment #1.1: Type: text/plain, Size: 8569 bytes --]
[CC += groff]
Hi Andrew,
On 12/14/22 23:57, Andrew Pinski wrote:
> On Wed, Dec 14, 2022 at 2:46 PM Alejandro Colomar via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> Hi,
>>
>> I was rewriting the strncat(3) manual page, and when I tried to compile the
>> example program, I got a surprise from the compiler.
>>
>> Here goes the page:
>>
>>
>> strncat(3) Library Functions Manual strncat(3)
>>
>> NAME
>> strncat - concatenate a null‐padded character sequence into a
>> string
>>
>> LIBRARY
>> Standard C library (libc, -lc)
>>
>> SYNOPSIS
>> #include <string.h>
>>
>> char *strncat(char *restrict dst, const char src[restrict .sz],
>> size_t sz);
>>
>> DESCRIPTION
>> This function catenates the input character sequence contained in
>> a null‐padded fixed‐width buffer, into a string at the buffer
>> pointed to by dst. The programmer is responsible for allocating a
>> buffer large enough, that is, strlen(dst) + strnlen(src, sz) + 1.
>>
>> An implementation of this function might be:
>>
>> char *
>> strncat(char *restrict dst, const char *restrict src, size_t sz)
>> {
>> int len;
>> char *end;
>>
>> len = strnlen(src, sz);
>> end = dst + strlen(dst);
>> end = mempcpy(end, src, len);
>> *end = '\0';
>>
>> return dst;
>> }
>>
>> RETURN VALUE
>> strncat() returns dest.
>>
>> ATTRIBUTES
>> [...]
>>
>> STANDARDS
>> POSIX.1‐2001, POSIX.1‐2008, C89, C99, SVr4, 4.3BSD.
>>
>> CAVEATS
>> The name of this function is confusing. This function has no re‐
>> lation with strncpy(3).
>>
>> If the destination buffer is not large enough, the behavior is un‐
>> defined. See _FORTIFY_SOURCE in feature_test_macros(7).
>>
>> BUGS
>> This function can be very inefficient. Read about Shlemiel
>> the painter ⟨https://www.joelonsoftware.com/2001/12/11/
>> back-to-basics/⟩.
>>
>> EXAMPLES
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <string.h>
>>
>> int
>> main(void)
>> {
>> char buf[BUFSIZ];
>> size_t len;
>>
>> buf[0] = '\0'; // There’s no ’cpy’ function to this ’cat’.
>> strncat(buf, "Hello ", 6);
>> strncat(buf, "world", 42); // Padding null bytes ignored.
>> strncat(buf, "!", 1);
>> len = strlen(buf);
>> printf("[len = %zu]: <%s>\n", len, buf);
>>
>> exit(EXIT_SUCCESS);
>> }
>>
>> SEE ALSO
>> string(3), string_copy(3)
>>
>> Linux man‐pages (unreleased) (date) strncat(3)
>>
>>
>> And when you compile that, you get:
>>
>> $ cc -Wall -Wextra ./strncat.c
>> ./strncat.c: In function ‘main’:
>> ./strncat.c:12:12: warning: ‘strncat’ specified bound 6 equals source length
>> [-Wstringop-overflow=]
>> 12 | strncat(buf, "Hello ", 6);
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~
>> ./strncat.c:14:12: warning: ‘strncat’ specified bound 1 equals source length
>> [-Wstringop-overflow=]
>> 14 | strncat(buf, "!", 1);
>> | ^~~~~~~~~~~~~~~~~~~~
>>
>>
>> So, what? Where's the problem? This function does exactly that: "take an
>> unterminated character sequence and catenate it to an existing string". Clang
>> seems to be fine with the code.
>
> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83404 and the
> background of why the warning was added here:
>
> https://www.us-cert.gov/bsi/articles/knowledge/coding-practices/strncpy-and-strncat.
This document is bogus, since it's puting strncpy(3) and strncat(3) in the same
sack, when they're in reality two completely different beasts. I'll quote below
some paragraphs of some new page I'm writing, which will show why.
The rationale behind GCC's warning is also fundamentally wrong. Martin was
wrong when he claimed that the right call for strncat(3) is the remaining space
in the destination.
I admit that I didn't know what strncat(3) was useful for, and believed that it
was simply a broken-by-design function until very recently (this week, more or
less). And to be honest, I still believe it's broken by design; it's just that
it can be repurposed for a reasonable new purpose (which I found while digging
in groff's source code; that's why the CC).
First I'll show an example program that I added to the strncat(3) manual page
last week, which is based on the groff code that used it:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define nitems(arr) (sizeof((arr)) / sizeof((arr)[0]))
int
main(void)
{
char pre[4] = "pre.";
char *post = ".post";
char *src = "some_long_body.post";
char dest[100];
dest[0] = '\0';
strncat(dest, pre, nitems(pre));
strncat(dest, src, strlen(src) - strlen(post));
puts(dest); // "pre.some_long_body"
exit(EXIT_SUCCESS);
}
And now I'll quote some text that I'm writing currently for the function:
Null‐padded character sequences
For historic reasons, some standard APIs, such as utmpx(5), use null‐
padded character sequences in fixed‐width buffers. To interface with
them, specialized functions need to be used.
To copy strings into them, use stpncpy(3).
To copy from an unterminated string within a fixed‐width buffer into a
string, ignoring any trailing null bytes in the source fixed‐width
buffer, you should use strncat(3).
[...]
stpncpy(3)
This function copies the input string into a destination
null‐padded character sequence in a fixed‐width buffer. If
the destination buffer, limited by its size, isn’t large
enough to hold the copy, the resulting character sequence
is truncated. Since it creates a character sequence, it
doesn’t need to write a terminating null byte. It’s impos‐
sible to distinguish truncation after the call, from a
character sequence that just fits the destination buffer;
truncation should be detected from the length of the origi‐
nal string.
strncpy(3)
This function is identical to stpncpy(3) except for the
useless return value.
stpncpy(3) is a simpler alternative to this function.
[...]
strncat(3)
Do not confuse this function with strncpy(3); they are not
related at all.
This function catenates the input character sequence con‐
tained in a null‐padded wixed‐width buffer, into a destina‐
tion string. The programmer is responsible for allocating
a buffer large enough. The return value is useless.
zustr2stp(3) is a faster alternative to this function.
An implementation of this function might be:
char *
strncat(char *restrict dst, const char *restrict src,
size_t sz)
{
int len;
char *end;
len = strnlen(src, sz);
end = dst + strlen(dst);
end = mempcpy(end, src, len);
*end = '\0';
return dst;
}
Cheers,
Alex
>
> Thanks,
> Andrew Pinski
>
>>
>> Cheers,
>>
>> Alex
>>
>>
>> --
>> <http://www.alejandro-colomar.es/>
--
<http://www.alejandro-colomar.es/>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [-Wstringop-overflow=] strncat(3)
2022-12-14 23:14 ` Alejandro Colomar
@ 2022-12-15 20:50 ` Martin Sebor
2022-12-15 22:03 ` Alejandro Colomar
0 siblings, 1 reply; 7+ messages in thread
From: Martin Sebor @ 2022-12-15 20:50 UTC (permalink / raw)
To: Alejandro Colomar, Andrew Pinski
Cc: gcc, linux-man, GNU C Library, groff, Martin Sebor
On 12/14/22 16:14, Alejandro Colomar via Libc-alpha wrote:
> [CC += groff]
>
> Hi Andrew,
>
> On 12/14/22 23:57, Andrew Pinski wrote:
>> On Wed, Dec 14, 2022 at 2:46 PM Alejandro Colomar via Libc-alpha
>> <libc-alpha@sourceware.org> wrote:
>>>
>>> Hi,
>>>
>>> I was rewriting the strncat(3) manual page, and when I tried to
>>> compile the
>>> example program, I got a surprise from the compiler.
>>>
>>> Here goes the page:
>>>
>>>
>>> strncat(3) Library Functions Manual
>>> strncat(3)
>>>
>>> NAME
>>> strncat - concatenate a null‐padded character
>>> sequence into a
>>> string
>>>
>>> LIBRARY
>>> Standard C library (libc, -lc)
>>>
>>> SYNOPSIS
>>> #include <string.h>
>>>
>>> char *strncat(char *restrict dst, const char src[restrict
>>> .sz],
>>> size_t sz);
>>>
>>> DESCRIPTION
>>> This function catenates the input character sequence
>>> contained in
>>> a null‐padded fixed‐width buffer, into a string at
>>> the buffer
>>> pointed to by dst. The programmer is responsible for
>>> allocating a
>>> buffer large enough, that is, strlen(dst) + strnlen(src,
>>> sz) + 1.
>>>
>>> An implementation of this function might be:
>>>
>>> char *
>>> strncat(char *restrict dst, const char *restrict src,
>>> size_t sz)
>>> {
>>> int len;
>>> char *end;
>>>
>>> len = strnlen(src, sz);
>>> end = dst + strlen(dst);
>>> end = mempcpy(end, src, len);
>>> *end = '\0';
>>>
>>> return dst;
>>> }
>>>
>>> RETURN VALUE
>>> strncat() returns dest.
>>>
>>> ATTRIBUTES
>>> [...]
>>>
>>> STANDARDS
>>> POSIX.1‐2001, POSIX.1‐2008, C89, C99, SVr4, 4.3BSD.
>>>
>>> CAVEATS
>>> The name of this function is confusing. This function
>>> has no re‐
>>> lation with strncpy(3).
>>>
>>> If the destination buffer is not large enough, the
>>> behavior is un‐
>>> defined. See _FORTIFY_SOURCE in feature_test_macros(7).
>>>
>>> BUGS
>>> This function can be very inefficient. Read about
>>> Shlemiel
>>> the painter
>>> ⟨https://www.joelonsoftware.com/2001/12/11/
>>> back-to-basics/⟩.
>>>
>>> EXAMPLES
>>> #include <stdio.h>
>>> #include <stdlib.h>
>>> #include <string.h>
>>>
>>> int
>>> main(void)
>>> {
>>> char buf[BUFSIZ];
>>> size_t len;
>>>
>>> buf[0] = '\0'; // There’s no ’cpy’ function to this
>>> ’cat’.
>>> strncat(buf, "Hello ", 6);
There's nothing wrong with this but the two lines above would be
more simply coded as one:
strcpy(buf, "Hello ");
The original code suggests a misunderstanding of strncpy's purpose:
that it writes exactly 6 bytes into the destination. That's what
the warning points out.
>>> strncat(buf, "world", 42); // Padding null bytes
>>> ignored.
>>> strncat(buf, "!", 1);
>>> len = strlen(buf);
>>> printf("[len = %zu]: <%s>\n", len, buf);
>>>
>>> exit(EXIT_SUCCESS);
>>> }
>>>
>>> SEE ALSO
>>> string(3), string_copy(3)
>>>
>>> Linux man‐pages (unreleased) (date)
>>> strncat(3)
>>>
>>>
>>> And when you compile that, you get:
>>>
>>> $ cc -Wall -Wextra ./strncat.c
>>> ./strncat.c: In function ‘main’:
>>> ./strncat.c:12:12: warning: ‘strncat’ specified bound 6 equals source
>>> length
>>> [-Wstringop-overflow=]
>>> 12 | strncat(buf, "Hello ", 6);
>>> | ^~~~~~~~~~~~~~~~~~~~~~~~~
>>> ./strncat.c:14:12: warning: ‘strncat’ specified bound 1 equals source
>>> length
>>> [-Wstringop-overflow=]
>>> 14 | strncat(buf, "!", 1);
>>> | ^~~~~~~~~~~~~~~~~~~~
>>>
>>>
>>> So, what? Where's the problem? This function does exactly that:
>>> "take an
>>> unterminated character sequence and catenate it to an existing
>>> string".
Strncat has historically had two distinct use cases. One of them
-- to constrain the amount of data to copy to the space remaining
in the destination -- gained popularity with the push to reduce
buffer overflow weaknesses in code. Mistakes in these uses gave
rise to a whole other class of security bugs, to the extent that
CERT felt it necessary to publish the strncpy and strncat best
practice. The GCC warning in turn was added to support the CERT
guideline. I touch on some of this in a blog post I wrote a few
years ago:
https://developers.redhat.com/blog/2018/05/24/detecting-string-truncation-with-gcc-8
The specific uses of the function above are contrived (there's
no point in calling strncat to append the full string -- strcat
will do that more clearly and efficiently) but the general use
case -- limiting the amount of copied data to an initial
substring of the source sequence -- although valid and originally
intended (it's one of the two uses of the function in UNIX v7),
is not one that either the guideline or the warning consider.
They can only consider one use cases, and they chose the one
that was observed behind security bugs. That choice unavoidably
leads to some false positives. The expected way to deal with
them is to suppress the warning by one of the usual mechanisms
(command line option or #pragma GCC diagnostic).
Martin
>>> Clang
>>> seems to be fine with the code.
>>
>> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83404 and the
>> background of why the warning was added here:
>>
>> https://www.us-cert.gov/bsi/articles/knowledge/coding-practices/strncpy-and-strncat.
>>
>
> This document is bogus, since it's puting strncpy(3) and strncat(3) in
> the same sack, when they're in reality two completely different beasts.
> I'll quote below some paragraphs of some new page I'm writing, which
> will show why.
>
> The rationale behind GCC's warning is also fundamentally wrong. Martin
> was wrong when he claimed that the right call for strncat(3) is the
> remaining space in the destination.
>
> I admit that I didn't know what strncat(3) was useful for, and believed
> that it was simply a broken-by-design function until very recently (this
> week, more or less). And to be honest, I still believe it's broken by
> design; it's just that it can be repurposed for a reasonable new purpose
> (which I found while digging in groff's source code; that's why the CC).
>
>
> First I'll show an example program that I added to the strncat(3) manual
> page last week, which is based on the groff code that used it:
>
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
>
> #define nitems(arr) (sizeof((arr)) / sizeof((arr)[0]))
>
> int
> main(void)
> {
> char pre[4] = "pre.";
> char *post = ".post";
> char *src = "some_long_body.post";
> char dest[100];
>
> dest[0] = '\0';
> strncat(dest, pre, nitems(pre));
> strncat(dest, src, strlen(src) - strlen(post));
>
> puts(dest); // "pre.some_long_body"
> exit(EXIT_SUCCESS);
> }
>
>
> And now I'll quote some text that I'm writing currently for the function:
>
>
> Null‐padded character sequences
> For historic reasons, some standard APIs, such as utmpx(5),
> use null‐
> padded character sequences in fixed‐width buffers. To
> interface with
> them, specialized functions need to be used.
>
> To copy strings into them, use stpncpy(3).
>
> To copy from an unterminated string within a fixed‐width buffer
> into a
> string, ignoring any trailing null bytes in the source
> fixed‐width
> buffer, you should use strncat(3).
>
> [...]
>
> stpncpy(3)
> This function copies the input string into a destination
> null‐padded character sequence in a fixed‐width buffer. If
> the destination buffer, limited by its size, isn’t large
> enough to hold the copy, the resulting character sequence
> is truncated. Since it creates a character sequence, it
> doesn’t need to write a terminating null byte. It’s impos‐
> sible to distinguish truncation after the call, from a
> character sequence that just fits the destination buffer;
> truncation should be detected from the length of the origi‐
> nal string.
>
> strncpy(3)
> This function is identical to stpncpy(3) except for the
> useless return value.
>
> stpncpy(3) is a simpler alternative to this function.
>
> [...]
>
> strncat(3)
> Do not confuse this function with strncpy(3); they are not
> related at all.
>
> This function catenates the input character sequence con‐
> tained in a null‐padded wixed‐width buffer, into a destina‐
> tion string. The programmer is responsible for allocating
> a buffer large enough. The return value is useless.
>
> zustr2stp(3) is a faster alternative to this function.
>
> An implementation of this function might be:
>
> char *
> strncat(char *restrict dst, const char *restrict src,
> size_t sz)
> {
> int len;
> char *end;
>
> len = strnlen(src, sz);
> end = dst + strlen(dst);
> end = mempcpy(end, src, len);
> *end = '\0';
>
> return dst;
> }
>
>
> Cheers,
>
> Alex
>
>
>
>>
>> Thanks,
>> Andrew Pinski
>>
>>>
>>> Cheers,
>>>
>>> Alex
>>>
>>>
>>> --
>>> <http://www.alejandro-colomar.es/>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [-Wstringop-overflow=] strncat(3)
2022-12-15 20:50 ` Martin Sebor
@ 2022-12-15 22:03 ` Alejandro Colomar
0 siblings, 0 replies; 7+ messages in thread
From: Alejandro Colomar @ 2022-12-15 22:03 UTC (permalink / raw)
To: Martin Sebor, Andrew Pinski
Cc: gcc, linux-man, GNU C Library, groff, Martin Sebor
[-- Attachment #1.1: Type: text/plain, Size: 7933 bytes --]
Hi Martin,
On 12/15/22 21:50, Martin Sebor wrote:
> On 12/14/22 16:14, Alejandro Colomar via Libc-alpha wrote:
[...]
>>>>
>>>> int
>>>> main(void)
>>>> {
>>>> char buf[BUFSIZ];
>>>> size_t len;
>>>>
>>>> buf[0] = '\0'; // There’s no ’cpy’ function to this ’cat’.
>>>> strncat(buf, "Hello ", 6);
>
> There's nothing wrong with this but the two lines above would be
> more simply coded as one:
>
> strcpy(buf, "Hello ");
I used a string literal to avoid having to think hard of an example using
utmpx(5), which would be the actual code for which I want users to use this
function.
With time, I expect to develop such example program.
>
> The original code suggests a misunderstanding of strncpy's purpose:
By the "original code" what are you exactly referring to? The code quoted
above, or another?
In any case, the code is using strncat(3) not strncpy(3).
> that it writes exactly 6 bytes into the destination.
That's the purpose of strncpy(3). I'll quote the versions of the pages that I'm
proposing:
strncpy(3):
These functions copy the string pointed to by src into a null‐padded
character sequence at the fixed‐width buffer pointed to by dst. If the
destination buffer, limited by its size, isn’t large enough to hold the
copy, the resulting character sequence is truncated. They only differ
in the return value.
The description above says that the destination buffer is limited by its size,
so as you say, this function copies exactly 6 bytes (the character sequence +
null padding).
strncat(3)
This function catenates the input character sequence contained in a
null‐padded fixed‐width buffer, into a string at the buffer pointed to
by dst. The programmer is responsible for allocating a buffer large
enough, that is, strlen(dst) + strnlen(src, sz) + 1.
But strncat(3) writes whatever is the length of the character sequence passed as
input (ignoring the padding null bytes), plus a terminating null byte.
> That's what
> the warning points out.
The warning would be fair for strncpy(3), but not for strncat(3).
>
>>>> strncat(buf, "world", 42); // Padding null bytes ignored.
>>>> strncat(buf, "!", 1);
>>>> len = strlen(buf);
>>>> printf("[len = %zu]: <%s>\n", len, buf);
>>>>
>>>> exit(EXIT_SUCCESS);
>>>> }
>>>>
>>>> SEE ALSO
>>>> string(3), string_copy(3)
>>>>
>>>> Linux man‐pages (unreleased) (date) strncat(3)
>>>>
>>>>
>>>> And when you compile that, you get:
>>>>
>>>> $ cc -Wall -Wextra ./strncat.c
>>>> ./strncat.c: In function ‘main’:
>>>> ./strncat.c:12:12: warning: ‘strncat’ specified bound 6 equals source length
>>>> [-Wstringop-overflow=]
>>>> 12 | strncat(buf, "Hello ", 6);
>>>> | ^~~~~~~~~~~~~~~~~~~~~~~~~
>>>> ./strncat.c:14:12: warning: ‘strncat’ specified bound 1 equals source length
>>>> [-Wstringop-overflow=]
>>>> 14 | strncat(buf, "!", 1);
>>>> | ^~~~~~~~~~~~~~~~~~~~
>>>>
>>>>
>>>> So, what? Where's the problem? This function does exactly that: "take an
>>>> unterminated character sequence and catenate it to an existing string".
>
> Strncat has historically had two distinct use cases. One of them
> -- to constrain the amount of data to copy to the space remaining
> in the destination -- gained popularity with the push to reduce
> buffer overflow weaknesses in code. Mistakes in these uses gave
> rise to a whole other class of security bugs, to the extent that
> CERT felt it necessary to publish the strncpy and strncat best
> practice. The GCC warning in turn was added to support the CERT
> guideline. I touch on some of this in a blog post I wrote a few
> years ago:
>
> https://developers.redhat.com/blog/2018/05/24/detecting-string-truncation-with-gcc-8
While this is true, that's because there was nothing more appropriate, such as
strlcat(3).
My point is that strncat(3) is far from being a good tool for that, and should
never be used for that purpose in good code; especially now that we have
strlcat(3) widely available in most if not all POSIX systems (in Linux through
libbsd).
I'll quote some part of the link above:
| strncat (dest, src, dest_size - strlen (dest) - 1);
|
| Calls that have this form are not diagnosed.
That code is a source of bugs. Such code should _always_ be replaced by
strlcat(3) as:
strlcat(dst, src, sizeof(dst);
| Other calls, such as those where
| the size is derived in some way from the size or length of the source string,
| are diagnosed by -Wstringop-overflow. That includes unsafe calls like
The only sane use of strncat(3) is that. It's a use where no other function
serves better. To read a possibly non-terminated character sequence from a
fixed-width buffer; and the size is the only way strncat(3) can know when to
stop reading.
|
| strncat (dest, src, strlen (src)); // incorrect - warning
This call doesn't make any sense; I agree. It should _always_ be replaced by
strcat(3) as:
strcat(dst, src);
I acknowledge that this is the case I show in the manual page, but only because
there is no form to create an unterminated string literal (I take this
opportunity to re-suggest "foo"u as an unterminated string literal).
I'll try to come up with an example program that avoids this.
|
| and
|
| strncat (dest, src, sizeof src); // incorrect - warning
This is the _perfect_ use case for strncat(3). No other function is better.
Let's say you have the following:
char src[5] = {'h', 'e', 'l', 'l', 'o'};
How do you copy that into a string? strncat(3). utmpx(5) is one of the
examples where this happens.
I'll modify the example program to use this, to avoid being misleading. I'll
replace string literals by character sequences.
>
> The specific uses of the function above are contrived (there's
> no point in calling strncat to append the full string -- strcat
> will do that more clearly and efficiently) but the general use
> case -- limiting the amount of copied data to an initial
> substring of the source sequence -- although valid and originally
> intended (it's one of the two uses of the function in UNIX v7),
> is not one that either the guideline or the warning consider.
> They can only consider one use cases, and they chose the one
> that was observed behind security bugs.
It's sad that strncat(3) has been used so much for limiting the dest copy. It's
pretty unsafe for that, we agree. How about allowing the safe use of it (i.e.,
size derived from source buffer), and warn about uses where the size is derived
from the dst string? I'm going to rewrite the manual page in that direction.
> That choice unavoidably
> leads to some false positives. The expected way to deal with
> them is to suppress the warning by one of the usual mechanisms
> (command line option or #pragma GCC diagnostic).
The only use case that GCC is allowing is better served by strlcpy(3) and
strlcat(3). I suggest changing the warning to protect the only natural use case
for strncat(3), and warn about uses of it supplanting strlcat(3) (and maybe
suggest using strlcat(3)).
>
> Martin
Thanks!
Alex
--
<http://www.alejandro-colomar.es/>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-12-15 22:03 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-14 22:45 [-Wstringop-overflow=] strncat(3) Alejandro Colomar
2022-12-14 22:51 ` Alejandro Colomar
2022-12-14 22:51 ` Alejandro Colomar
2022-12-14 22:57 ` Andrew Pinski
2022-12-14 23:14 ` Alejandro Colomar
2022-12-15 20:50 ` Martin Sebor
2022-12-15 22:03 ` Alejandro Colomar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).