Use strlcpy(3) instead of its pattern

-  Since strncpy(3) is not designed to write strings, but rather
   (null-padded) character sequences (a.k.a. unterminated strings), we
   had to manually append a '\0'.  strlcpy(3) creates strings, so they
   are always terminated.  This removes dependencies between lines, and
   also removes chances of accidents.

-  Repurposing strncpy(3) to create strings requires calculating the
   location of the terminating null byte, which involves a '-1'
   calculation.  This is a source of off-by-one bugs.  The new code has
   no '-1' calculations, so there's almost-zero chance of these bugs.

-  strlcpy(3) doesn't padd with null bytes.  Padding is relevant when
   writing fixed-width buffers to binary files, when interfacing certain
   APIs (I believe utmpx requires null padding at lease in some
   systems), or when sending them to other processes or through the
   network.  This is not the case, so padding is effectively ignored.

-  strlcpy(3) requires that the input string is really a string;
   otherwise it crashes (SIGSEGV).  Let's check if the input strings are
   really strings:

   -  lib/fields.c:
      -  'cp' was assigned from 'newft', and 'newft' comes from fgets(3).

   -  lib/gshadow.c:
      -  strlen(string) is calculated a few lines above.

   -  libmisc/console.c:
      -  'cons' comes from getdef_str, which is a bit cryptic, but seems
         to generate strings, I guess.1

   -  libmisc/date_to_str.c:
      -  It receives a string literal.  :)

   -  libmisc/utmp.c:
      -  'tname' comes from ttyname(3), which returns a string.

   -  src/su.c:
      -  'tmp_name' has been passed to strcmp(3) a few lines above.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
This commit is contained in:
Alejandro Colomar 2022-12-16 04:13:53 +01:00 committed by Serge Hallyn
parent a48d77bdef
commit 220b352b70
6 changed files with 12 additions and 16 deletions

View File

@ -100,8 +100,7 @@ void change_field (char *buf, size_t maxsize, const char *prompt)
cp++;
}
strncpy (buf, cp, maxsize - 1);
buf[maxsize - 1] = '\0';
strlcpy (buf, cp, maxsize);
}
}

View File

@ -15,6 +15,7 @@
#ident "$Id$"
#include <stdio.h>
#include <string.h>
#include "prototypes.h"
#include "defines.h"
static /*@null@*/FILE *shadow;
@ -124,8 +125,7 @@ void endsgent (void)
sgrbuflen = len;
}
strncpy (sgrbuf, string, len);
sgrbuf[len-1] = '\0';
strlcpy (sgrbuf, string, len);
cp = strrchr (sgrbuf, '\n');
if (NULL != cp) {

View File

@ -44,8 +44,7 @@ static bool is_listed (const char *cfgin, const char *tty, bool def)
if (*cons != '/') {
char *pbuf;
strncpy (buf, cons, sizeof (buf));
buf[sizeof (buf) - 1] = '\0';
strlcpy (buf, cons, sizeof (buf));
pbuf = &buf[0];
while ((s = strtok (pbuf, ":")) != NULL) {
if (strcmp (s, tty) == 0) {

View File

@ -38,9 +38,10 @@ void date_to_str (size_t size, char buf[size], long date)
time_t t;
t = date;
if (date < 0)
(void) strncpy (buf, "never", size);
else
if (date < 0) {
(void) strlcpy (buf, "never", size);
} else {
(void) strftime (buf, size, "%Y-%m-%d", gmtime (&t));
buf[size - 1] = '\0';
buf[size - 1] = '\0';
}
}

View File

@ -40,10 +40,8 @@ static bool is_my_tty (const char *tty)
if ('\0' == tmptty[0]) {
const char *tname = ttyname (STDIN_FILENO);
if (NULL != tname) {
(void) strncpy (tmptty, tname, sizeof tmptty);
tmptty[sizeof (tmptty) - 1] = '\0';
}
if (NULL != tname)
(void) strlcpy (tmptty, tname, sizeof tmptty);
}
if ('\0' == tmptty[0]) {

View File

@ -653,8 +653,7 @@ static /*@only@*/struct passwd * check_perms (void)
SYSLOG ((LOG_INFO,
"Change user from '%s' to '%s' as requested by PAM",
name, tmp_name));
strncpy (name, tmp_name, sizeof(name) - 1);
name[sizeof(name) - 1] = '\0';
strlcpy (name, tmp_name, sizeof(name));
pw = xgetpwnam (name);
if (NULL == pw) {
(void) fprintf (stderr,