pam_issue: fix potential read out of bounds

Reported by gcc-10 -Warray-bounds:

In file included from /usr/include/string.h:494,
                 from modules/pam_issue/pam_issue.c:19:
In function 'strncat',
    inlined from 'read_issue_quoted' at modules/pam_issue/pam_issue.c:197:3:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:136:10: error: '__builtin___strncat_chk' offset [260, 389] from the object at 'uts' is out of the bounds of referenced subobject 'version' with type 'char[65]' at offset 195 [-Werror=array-bounds]
  136 |   return __builtin___strncat_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from modules/pam_issue/pam_issue.c:26:
modules/pam_issue/pam_issue.c: In function 'read_issue_quoted':
/usr/include/x86_64-linux-gnu/sys/utsname.h:59:10: note: subobject 'version' declared here
   59 |     char version[_UTSNAME_VERSION_LENGTH];
      |          ^~~~~~~
In file included from /usr/include/string.h:494,
                 from modules/pam_issue/pam_issue.c:19:
In function 'strncat',
    inlined from 'read_issue_quoted' at modules/pam_issue/pam_issue.c:188:3:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:136:10: error: '__builtin___strncat_chk' offset [65, 389] from the object at 'uts' is out of the bounds of referenced subobject 'sysname' with type 'char[65]' at offset 0 [-Werror=array-bounds]
  136 |   return __builtin___strncat_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from modules/pam_issue/pam_issue.c:26:
modules/pam_issue/pam_issue.c: In function 'read_issue_quoted':
/usr/include/x86_64-linux-gnu/sys/utsname.h:51:10: note: subobject 'sysname' declared here
   51 |     char sysname[_UTSNAME_SYSNAME_LENGTH];
      |          ^~~~~~~
In file included from /usr/include/string.h:494,
                 from modules/pam_issue/pam_issue.c:19:
In function 'strncat',
    inlined from 'read_issue_quoted' at modules/pam_issue/pam_issue.c:194:3:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:136:10: error: '__builtin___strncat_chk' offset [195, 389] from the object at 'uts' is out of the bounds of referenced subobject 'release' with type 'char[65]' at offset 130 [-Werror=array-bounds]
  136 |   return __builtin___strncat_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from modules/pam_issue/pam_issue.c:26:
modules/pam_issue/pam_issue.c: In function 'read_issue_quoted':
/usr/include/x86_64-linux-gnu/sys/utsname.h:57:10: note: subobject 'release' declared here
   57 |     char release[_UTSNAME_RELEASE_LENGTH];
      |          ^~~~~~~
In file included from /usr/include/string.h:494,
                 from modules/pam_issue/pam_issue.c:19:
In function 'strncat',
    inlined from 'read_issue_quoted' at modules/pam_issue/pam_issue.c:191:3:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:136:10: error: '__builtin___strncat_chk' offset [130, 389] from the object at 'uts' is out of the bounds of referenced subobject 'nodename' with type 'char[65]' at offset 65 [-Werror=array-bounds]
  136 |   return __builtin___strncat_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from modules/pam_issue/pam_issue.c:26:
modules/pam_issue/pam_issue.c: In function 'read_issue_quoted':
/usr/include/x86_64-linux-gnu/sys/utsname.h:54:10: note: subobject 'nodename' declared here
   54 |     char nodename[_UTSNAME_NODENAME_LENGTH];
      |          ^~~~~~~~
In file included from /usr/include/string.h:494,
                 from modules/pam_issue/pam_issue.c:19:
In function 'strncat',
    inlined from 'read_issue_quoted' at modules/pam_issue/pam_issue.c:200:3:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:136:10: error: '__builtin___strncat_chk' offset [325, 389] from the object at 'uts' is out of the bounds of referenced subobject 'machine' with type 'char[65]' at offset 260 [-Werror=array-bounds]
  136 |   return __builtin___strncat_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from modules/pam_issue/pam_issue.c:26:
modules/pam_issue/pam_issue.c: In function 'read_issue_quoted':
/usr/include/x86_64-linux-gnu/sys/utsname.h:62:10: note: subobject 'machine' declared here
   62 |     char machine[_UTSNAME_MACHINE_LENGTH];
      |          ^~~~~~~

* modules/pam_issue/pam_issue.c (read_issue_quoted): Rewrite to avoid
strncat from potentially not null-terminated string buffer fields
of struct utsname.

1 files changed, 29 insertions, 21 deletions

diff --git a/modules/pam_issue/pam_issue.c b/modules/pam_issue/pam_issue.c
index 49cbad4c..8a74ce03 100644
--- a/modules/pam_issue/pam_issue.c
+++ b/modules/pam_issue/pam_issue.c
@@ -163,6 +163,7 @@ read_issue_quoted(pam_handle_t *pamh, FILE *fp, char **prompt)
 {
     int c;
     size_t size = 1024;
+    size_t issue_len = 0;
     char *issue;
     struct utsname uts;
 
@@ -173,42 +174,41 @@ read_issue_quoted(pam_handle_t *pamh, FILE *fp, char **prompt)
 	return PAM_BUF_ERR;
     }
 
-    issue[0] = '\0';
     (void) uname(&uts);
 
     while ((c = getc(fp)) != EOF) {
-	char buf[1024];
+	const char *src = NULL;
+	size_t len = 0;
+	char buf[1024] = "";
 
-	buf[0] = '\0';
 	if (c == '\\') {
 	    if ((c = getc(fp)) == EOF)
 		break;
 	    switch (c) {
 	      case 's':
-		strncat(buf, uts.sysname, sizeof(buf) - 1);
+		src = uts.sysname;
+		len = strnlen(uts.sysname, sizeof(uts.sysname));
 		break;
 	      case 'n':
-		strncat(buf, uts.nodename, sizeof(buf) - 1);
+		src = uts.nodename;
+		len = strnlen(uts.nodename, sizeof(uts.nodename));
 		break;
 	      case 'r':
-		strncat(buf, uts.release, sizeof(buf) - 1);
+		src = uts.release;
+		len = strnlen(uts.release, sizeof(uts.release));
 		break;
 	      case 'v':
-		strncat(buf, uts.version, sizeof(buf) - 1);
+		src = uts.version;
+		len = strnlen(uts.version, sizeof(uts.version));
 		break;
 	      case 'm':
-		strncat(buf, uts.machine, sizeof(buf) - 1);
+		src = uts.machine;
+		len = strnlen(uts.machine, sizeof(uts.machine));
 		break;
 	      case 'o':
 #ifdef HAVE_GETDOMAINNAME
-		{
-		    char domainname[256];
-
-		    if (getdomainname(domainname, sizeof(domainname)) >= 0) {
-			domainname[sizeof(domainname)-1] = '\0';
-			strncat(buf, domainname, sizeof(buf) - 1);
-		    }
-		}
+		if (getdomainname(buf, sizeof(buf)) >= 0)
+		    buf[sizeof(buf) - 1] = '\0';
 #endif
 		break;
 	      case 'd':
@@ -243,7 +243,8 @@ read_issue_quoted(pam_handle_t *pamh, FILE *fp, char **prompt)
 			const char *str = pam_str_skip_prefix(ttyn, "/dev/");
 			if (str != NULL)
 			    ttyn = str;
-			strncat(buf, ttyn, sizeof(buf) - 1);
+			src = ttyn;
+			len = strlen(ttyn);
 		    }
 		}
 		break;
@@ -272,20 +273,27 @@ read_issue_quoted(pam_handle_t *pamh, FILE *fp, char **prompt)
 	    buf[0] = c; buf[1] = '\0';
 	}
 
-	if ((strlen(issue) + strlen(buf)) + 1 > size) {
+	if (src == NULL) {
+	    src = buf;
+	    len = strlen(buf);
+	}
+	if (issue_len + len + 1 > size) {
 	    char *new_issue;
 
-	    size += strlen(buf) + 1;
-	    new_issue = (char *) realloc (issue, size);
+	    size += len + 1;
+	    new_issue = realloc (issue, size);
 	    if (new_issue == NULL) {
 		_pam_drop(issue);
 		return PAM_BUF_ERR;
 	    }
 	    issue = new_issue;
 	}
-	strcat(issue, buf);
+	memcpy(issue + issue_len, src, len);
+	issue_len += len;
     }
 
+    issue[issue_len] = '\0';
+
     if (ferror(fp)) {
 	pam_syslog(pamh, LOG_ERR, "read error: %m");
 	_pam_drop(issue);