Changed the way the "when" variable is used internally in shutdown.c.

It starts as a NULL pointer, then might get set as a pointer to optarg,
then it might get set to point to an argv parameter, then it might have
a string value copied into it, over-writing the original data. We should
not risk over-writing internal variables which might get used for something
else (it's rude and security risk). Set up "when" as its own buffer
that has data from optargs and/or argv copied into it.

Minor code fixes across multiple source files to avoid buffer
overflows, or uninitialized strings.

doc/Changelog

@@ -21,6 +21,17 @@ sysvinit (2.94) unreleased; urgency=low
     * Added logsave.c and logsave.8 manual page from e2fsprogs to make
       sure logsave is available to initscripts.
     * Updated src/Makefile to make sure bootlogd compiles with Clang.
+    * Use defined constants for password length in sulogin. Makes
+      it easier to update/patch later.
+    * Minor code fixes across multiple source files to avoid buffer
+      overflows, or uninitialized strings.
+    * Changed the way the "when" variable is used internally in shutdown.c.
+      It starts as a NULL pointer, then might get set as a pointer to optarg,
+      then it might get set to point to an argv parameter, then it might have
+      a string value copied into it, over-writing the original data. We should
+      not risk over-writing internal variables which might get used for something
+      else (it's rude and security risk). Set up "when" as its own buffer
+      that has data from optargs and/or argv copied into it.
 
 
 sysvinit (2.93) released; urgency=low

src/killall5.c

@@ -433,7 +433,11 @@ int check4nfs(const char * path, char * real)
 
 	} while (1);
 
-	if (real) strcpy(real, curr);
+	if (real)     /* real is defined elsewhere as being PATH_MAX + 1 */
+        {
+           memset(real, '\0', PATH_MAX + 1);
+           strncpy(real, curr, PATH_MAX);
+        }
 
 	if (errno == EINVAL) {
 		const size_t nlen = strlen(curr);

src/last.c

@@ -48,6 +48,10 @@
 #  define SHUTDOWN_TIME 254
 #endif
 
+#ifndef PATH_MAX
+#define PATH_MAX 2048
+#endif
+
 char *Version = "@(#) last 2.85 31-Apr-2004 miquels";
 
 #define CHOP_DOMAIN	0	/* Define to chop off local domainname. */
@@ -253,10 +257,11 @@ int uread(FILE *fp, struct utmp *u, int *quit)
 #define BTMP_FILE getbtmp()
 char *getbtmp()
 {
-	static char btmp[128];
+	static char btmp[PATH_MAX + 5];  /* max path + btmp + null terminator */
 	char *p;
 
-	strcpy(btmp, WTMP_FILE);
+        memset(btmp, '\0', PATH_MAX + 5);
+	strncpy(btmp, WTMP_FILE, PATH_MAX);
 	if ((p = strrchr(btmp, '/')) == NULL)
 		p = btmp;
 	else
@@ -841,7 +846,7 @@ int main(int argc, char **argv)
 	switch (ut.ut_type) {
 		case SHUTDOWN_TIME:
 			if (extended) {
-				strcpy(ut.ut_line, "system down");
+				strncpy(ut.ut_line, "system down", OLD_LINESIZE - 1);
 				quit = list(&ut, lastboot, R_NORMAL);
 			}
 			lastdown = lastrch = ut.ut_time;
@@ -850,14 +855,14 @@ int main(int argc, char **argv)
 		case OLD_TIME:
 		case NEW_TIME:
 			if (extended) {
-				strcpy(ut.ut_line,
+				strncpy(ut.ut_line,
 				ut.ut_type == NEW_TIME ? "new time" :
-					"old time");
+					"old time", OLD_LINESIZE - 1);
 				quit = list(&ut, lastdown, R_TIMECHANGE);
 			}
 			break;
 		case BOOT_TIME:
-			strcpy(ut.ut_line, "system boot");
+			strncpy(ut.ut_line, "system boot", OLD_LINESIZE - 1);
 			quit = list(&ut, lastdown, R_REBOOT);
 			lastboot = ut.ut_time;
 			down = 1;

src/logsave.c

@@ -282,7 +282,7 @@ int main(int argc, char **argv)
 	outfn = argv[optind];
 	optind++;
 	argv += optind;
-	argc -= optind;
+	/* argc -= optind;  - this is not used */
 
 	outfd = open(outfn, openflags, 0644);
 	do_stdin = !strcmp(argv[0], "-");

src/mountpoint.c

@@ -32,6 +32,10 @@
 #include <getopt.h>
 #include <stdio.h>
 
+#ifndef PATH_MAX
+#define PATH_MAX 2048
+#endif
+
 int dostat(char *path, struct stat *st, int do_lstat, int quiet)
 {
 	int		n;
@@ -105,7 +109,7 @@ void usage(void) {
 int main(int argc, char **argv)
 {
 	struct stat	st, st2;
-	char		buf[256];
+	char		buf[PATH_MAX + 1];
 	char		*path;
 	int		quiet = 0;
 	int		showdev = 0;
@@ -163,7 +167,7 @@ int main(int argc, char **argv)
 
 	memset(buf, 0, sizeof(buf));
 	strncpy(buf, path, sizeof(buf) - 4);
-	strcat(buf, "/..");
+	strncat(buf, "/..", 3);
 	if (dostat(buf, &st2, 0, quiet) < 0)
 		return 1;
 

src/shutdown.c

@@ -70,6 +70,8 @@ extern char **environ;
 #endif
 
 #define MESSAGELEN	256
+#define STATELEN        64
+#define WHEN_SIZE       64
 
 /* Whether we should warn system is shutting down */
 #define QUIET_FULL 2
@@ -83,7 +85,7 @@ int fastboot = 0;	/* Do a 'fast' reboot		*/
 int forcefsck = 0;	/* Force fsck on reboot		*/
 char message[MESSAGELEN];	/* Warning message	*/
 char *sltime = 0;	/* Sleep time			*/
-char newstate[64];	/* What are we gonna do		*/
+char newstate[STATELEN];	/* What are we gonna do		*/
 int doself = 0;		/* Don't use init		*/
 int got_alrm = 0;
 
@@ -232,11 +234,11 @@ int init_setenv(char *name, char *value)
  */
 void issue_warn(int mins)
 {
-	char buf[MESSAGELEN + sizeof(newstate)];
+	char buf[MESSAGELEN + sizeof(newstate) + 1];
 	int len;
 
 	buf[0] = 0;
-	strncat(buf, message, sizeof(buf) - 1);
+	strncpy(buf, message, MESSAGELEN);
 	len = strlen(buf);
 
 	if (mins == 0)
@@ -519,7 +521,7 @@ int main(int argc, char **argv)
 	char			buf[128];
 	char			term[UT_LINESIZE + 6];
 	char			*sp;
-	char			*when = NULL;
+	char			when[WHEN_SIZE];
 	int			c, i, wt;
 	int			hours, mins;
 	int			didnolog = 0;
@@ -547,6 +549,7 @@ int main(int argc, char **argv)
 	}
 	strcpy(down_level, "1");
 	halttype = NULL;
+        memset(when, '\0', WHEN_SIZE);
 
 	/* Process the options. */
 	while((c = getopt(argc, argv, "HPacqQkrhnfFyt:g:i:")) != EOF) {
@@ -593,7 +596,7 @@ int main(int argc, char **argv)
 			case 'y': /* Ignored for sysV compatibility */
 				break;
 			case 'g': /* sysv style to specify time. */
-				when = optarg;
+				strncpy(when, optarg, WHEN_SIZE - 1);
 				break;
 			case 'i': /* Level to go to. */
 				if (!strchr("0156aAbBcCsS", optarg[0])) {
@@ -679,7 +682,7 @@ int main(int argc, char **argv)
 
 	/* Read remaining words, skip time if needed. */
 	message[0] = 0;
-	for(c = optind + (!cancel && !when); c < argc; c++) {
+	for(c = optind + (!cancel && !when[0]); c < argc; c++) {
 		if (strlen(message) + strlen(argv[c]) + 4 > MESSAGELEN)
 			break;
   		strcat(message, argv[c]);
@@ -704,9 +707,9 @@ int main(int argc, char **argv)
 	}
   
 	/* Check syntax. */
-	if (when == NULL) {
+	if (when[0] == '\0') {
 		if (optind == argc) usage();
-		when = argv[optind++];
+                strncpy(when, argv[optind++], WHEN_SIZE - 1);
 	}
 
 	/* See if we are already running. */
@@ -725,16 +728,16 @@ int main(int argc, char **argv)
 	/* Tell users what we're gonna do. */
 	switch(down_level[0]) {
 		case '0':
-			strcpy(newstate, "for system halt");
+			strncpy(newstate, "for system halt", STATELEN);
 			break;
 		case '6':
-			strcpy(newstate, "for reboot");
+			strncpy(newstate, "for reboot", STATELEN);
 			break;
 		case '1':
-			strcpy(newstate, "to maintenance mode");
+			strncpy(newstate, "to maintenance mode", STATELEN);
 			break;
 		default:
-			sprintf(newstate, "to runlevel %s", down_level);
+			snprintf(newstate, STATELEN, "to runlevel %s", down_level);
 			break;
 	}
 
@@ -772,10 +775,11 @@ int main(int argc, char **argv)
 
 	/* Alias now and take care of old '+mins' notation. */
 	if (!strcmp(when, "now")) strcpy(when, "0");
-	if (when[0] == '+') when++;
 
+        sp = when;
+	if (when[0] == '+') sp++;
 	/* Decode shutdown time. */
-	for (sp = when; *sp; sp++) {
+	for ( ; *sp; sp++) {
 		if (*sp != ':' && (*sp < '0' || *sp > '9'))
 			usage();
 	}