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>

diff --git a/lib/fields.c b/lib/fields.c
index 8a5603525408ac472372d9b70cb73e8491adffcf..1002d9d8c6996534f5048676ec77356d1b40da3b 100644 (file)
--- a/lib/fields.c
+++ b/lib/fields.c
@@ -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);
        }
 }
 

diff --git a/lib/gshadow.c b/lib/gshadow.c
index cff8cb5828b70ca2702888f5c4b5167e93e055c8..f075aa97eb79c2aac2430fb9ecd24a3a5be944c1 100644 (file)
--- a/lib/gshadow.c
+++ b/lib/gshadow.c
@@ -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) {

diff --git a/libmisc/console.c b/libmisc/console.c
index c475aa23fd91a6645b96e3e6b1bcbdeb8167c524..bc024eba682384b59bdecc5dd3e13c5c3ef93aa9 100644 (file)
--- a/libmisc/console.c
+++ b/libmisc/console.c
@@ -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) {

diff --git a/libmisc/date_to_str.c b/libmisc/date_to_str.c
index 07e99f1a7e15f37ccb6e1d99ce66f80c1a858df4..f3b9dc766649daffefb4f923c1b829fec0576b91 100644 (file)
--- a/libmisc/date_to_str.c
+++ b/libmisc/date_to_str.c
@@ -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';
+       }
 }

diff --git a/libmisc/utmp.c b/libmisc/utmp.c
index 6662bbe9b3e594f2a1b81cc1a9bce726ae7662c0..e435c3220857ff16799c738821eedc28c6bf3b66 100644 (file)
--- a/libmisc/utmp.c
+++ b/libmisc/utmp.c
@@ -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]) {

diff --git a/src/su.c b/src/su.c
index b2f0378b9ea298bdccb9a142370cdb6b09eb0cb8..9578ab2b9227a6ed55a6a7ce338362b237c8e524 100644 (file)
--- a/src/su.c
+++ b/src/su.c
@@ -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,