From bb7411020a6afed4783e32e8c5c18520f3e7e0ab Mon Sep 17 00:00:00 2001
From: Marcin Nowakowski <marcin.w.nowakowski@gmail.com>
Date: Fri, 10 May 2024 09:18:51 +0200
Subject: [PATCH] Fix coverity unbound buffer issues

During coverity scan, there is reported an issue
with unbounded source buffer for each usage of input arg
directly with syslog function.
This commit is to limit the names of users and groups
passed as input args to a fixed value of 256.
The max length 256 is a typical POSIX login name length.
This should improve also security.
---
 lib/alloc.c   | 20 ++++++++++++++++++++
 lib/alloc.h   |  2 ++
 lib/defines.h |  9 +++++++++
 src/chfn.c    |  2 +-
 src/chsh.c    |  2 +-
 src/newgrp.c  |  4 ++--
 src/passwd.c  |  2 +-
 7 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/lib/alloc.c b/lib/alloc.c
index 962f45a1d..bb0a68ef5 100644
--- a/lib/alloc.c
+++ b/lib/alloc.c
@@ -71,3 +71,23 @@ xreallocarray(void *p, size_t nmemb, size_t size)
 	        log_get_progname(), strerror(errno));
 	exit(13);
 }
+
+char *
+xstrndup(const char *s, size_t max_length) {
+    size_t length = strlen(s);
+    if (length > max_length) {
+        length = max_length;
+    }
+
+    char *p = malloc(length + 1);
+    if (!p) {
+		fprintf(log_get_logfd(), _("%s: %s\n"),
+	    		log_get_progname(), strerror(errno));
+        exit(13);
+    }
+
+    strncpy(p, s, length);
+    p[length] = '\0';
+
+    return p;
+}
\ No newline at end of file
diff --git a/lib/alloc.h b/lib/alloc.h
index 39405a56f..dd8f7f4a0 100644
--- a/lib/alloc.h
+++ b/lib/alloc.h
@@ -55,6 +55,8 @@ void *xcalloc(size_t nmemb, size_t size);
 ATTR_MALLOC(free)
 void *xreallocarray(void *p, size_t nmemb, size_t size);
 
+ATTR_MALLOC(free)
+char *xstrndup(const char *s, size_t max_length);
 
 inline void *
 xmalloc(size_t size)
diff --git a/lib/defines.h b/lib/defines.h
index 8c55dddbc..4e701367c 100644
--- a/lib/defines.h
+++ b/lib/defines.h
@@ -213,4 +213,13 @@
 #define PASS_MAX  BUFSIZ - 1
 #endif
 
+/* Maximum length of user name or group name
+ * This is used only to limit a string length passed to syslog.
+ * An issue "unbounded source buffer" is reported while doing 
+ * coverity scans, when there is an unlimited string length passed 
+ * to syslog function.
+ * Typical POSIX login name max length is 256.
+ */
+#define USER_ENTRY_MAX_LENGTH 256
+
 #endif				/* _DEFINES_H_ */
diff --git a/src/chfn.c b/src/chfn.c
index 9043212a1..52717bb90 100644
--- a/src/chfn.c
+++ b/src/chfn.c
@@ -648,7 +648,7 @@ int main (int argc, char **argv)
 	 * name, or the name getlogin() returns.
 	 */
 	if (optind < argc) {
-		user = argv[optind];
+		user = xstrndup(argv[optind], USER_ENTRY_MAX_LENGTH);
 		pw = xgetpwnam (user);
 		if (NULL == pw) {
 			fprintf (stderr, _("%s: user '%s' does not exist\n"), Prog,
diff --git a/src/chsh.c b/src/chsh.c
index c4918c1b1..a6a1d3647 100644
--- a/src/chsh.c
+++ b/src/chsh.c
@@ -498,7 +498,7 @@ int main (int argc, char **argv)
 	 * name, or the name getlogin() returns.
 	 */
 	if (optind < argc) {
-		user = argv[optind];
+		user = xstrndup(argv[optind], USER_ENTRY_MAX_LENGTH);
 		pw = xgetpwnam (user);
 		if (NULL == pw) {
 			fprintf (stderr,
diff --git a/src/newgrp.c b/src/newgrp.c
index 1b3d76b83..c7070c10a 100644
--- a/src/newgrp.c
+++ b/src/newgrp.c
@@ -482,7 +482,7 @@ int main (int argc, char **argv)
 		 * not "newgrp".
 		 */
 		if ((argc > 0) && (argv[0][0] != '-')) {
-			group = argv[0];
+			group = xstrndup(argv[0], USER_ENTRY_MAX_LENGTH);
 			argc--;
 			argv++;
 		} else {
@@ -513,7 +513,7 @@ int main (int argc, char **argv)
 			usage ();
 			goto failure;
 		} else if (argv[0] != NULL) {
-			group = argv[0];
+			group = xstrndup(argv[0], USER_ENTRY_MAX_LENGTH);
 		} else {
 			/*
 			 * get the group file entry for her login group id.
diff --git a/src/passwd.c b/src/passwd.c
index 2999a3c88..93ded0c51 100644
--- a/src/passwd.c
+++ b/src/passwd.c
@@ -907,7 +907,7 @@ int main (int argc, char **argv)
 	}
 	myname = xstrdup (pw->pw_name);
 	if (optind < argc) {
-		name = argv[optind];
+		name = xstrndup(argv[optind], USER_ENTRY_MAX_LENGTH);
 	} else {
 		name = myname;
 	}