>Number:         23409
>Category:       bin
>Synopsis:       last -H ... can blindly print out non-NUL terminated strings
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Nov 11 05:21:00 UTC 2003
>Closed-Date:
>Last-Modified:
>Originator:     Geoff C. Wing
>Release:        NetBSD 1.6ZE (2003/11/11)
>Organization:
>Environment:
System: NetBSD g.primenet.com.au 1.6ZE NetBSD 1.6ZE (G) #0: Mon Nov 10 14:30:01 EST 2003 gcw@g.primenet.com.au:/usr/netbsd/src/sys/arch/i386/compile/G i386
Architecture: i386
Machine: i386
>Description:
	when querying a wtmp file, ``last -H <n>'' where <n> is greater than
	UT_HOSTSIZE will spew out data from outside the ut_host[] field if
	the ut_host[] entry is UT_HOSTSIZE long (and doesn't have a NUL
	terminator).  From inspection, the same is true of -L <n> and -N <n>
	options.  And IIRC also true of wtmpx files.

>How-To-Repeat:
	Do ``last -H 30 -f /var/log/wtmp'' on some host which has had logins
	from hosts with >=16 letter hostnames.
>Fix:
	If parameters passed are greater than field sizes then these fields
	need to be copied out and NUL-terminated before being printed.
	Something along the lines of the following:

--- want.c.org	2003-08-09 12:02:04.000000000 +1000
+++ want.c	2003-11-11 16:16:43.000000000 +1100
@@ -48,12 +48,29 @@
 	int	bytes, wfd;
 	char	*ct, *crmsg;
 	size_t  len = sizeof(*buf) * MAXUTMP;
+	int     checkname, checkline, checkhost;
+	char   *namebuf, *linebuf, *hostbuf;
 
 	if ((buf = malloc(len)) == NULL)
 		err(1, "Cannot allocate utmp buffer");
 
 	crmsg = NULL;
 
+	checkname = namesz > sizeof(bp->ut_name) ? 1 : 0;
+	checkline = linesz > sizeof(bp->ut_line) ? 1 : 0;
+	checkhost = hostsz > sizeof(bp->ut_host) ? 1 : 0;
+
+	if (checkname)
+		namebuf = malloc(namesz + 1);
+	if (checkline)
+		linebuf = malloc(linesz + 1);
+	if (checkhost)
+		hostbuf = malloc(hostsz + 1);
+	if ((checkname && namebuf == NULL)
+	    || (checkline && linebuf == NULL)
+	    || (checkhost && hostbuf == NULL))
+		err(1, "Cannot allocate buffer");
+
 	if ((wfd = open(file, O_RDONLY, 0)) < 0 || fstat(wfd, &stb) == -1)
 		err(1, "%s", file);
 	bl = (stb.st_size + len - 1) / len;
@@ -67,23 +84,45 @@
 		    (bytes = read(wfd, buf, len)) == -1)
 			err(1, "%s", file);
 		for (bp = &buf[bytes / sizeof(*buf) - 1]; bp >= buf; --bp) {
+			if (!checkname)
+				namebuf = bp->ut_name;
+			else {
+				strncpy(namebuf, bp->ut_name,
+					sizeof(bp->ut_name));
+				namebuf[sizeof(bp->ut_name)] = '\0';
+			}
+			if (!checkline)
+				linebuf = bp->ut_line;
+			else {
+				strncpy(linebuf, bp->ut_line,
+					sizeof(bp->ut_line));
+				linebuf[sizeof(bp->ut_line)] = '\0';
+			}
+			if (!checkhost)
+				hostbuf = bp->ut_host;
+			else {
+				strncpy(hostbuf, bp->ut_host,
+					sizeof(bp->ut_host));
+				hostbuf[sizeof(bp->ut_host)] = '\0';
+			}
+
 			/*
 			 * if the terminal line is '~', the machine stopped.
 			 * see utmp(5) for more info.
 			 */
-			if (bp->ut_line[0] == '~' && !bp->ut_line[1]) {
+			if (linebuf[0] == '~' && !linebuf[1]) {
 				/* everybody just logged out */
 				for (T = ttylist; T; T = T->next)
 					T->logout = -bp->ut_timefld;
 				currentout = -bp->ut_timefld;
-				crmsg = strncmp(bp->ut_name, "shutdown",
+				crmsg = strncmp(namebuf, "shutdown",
 				    namesz) ? "crash" : "shutdown";
 				if (want(bp, NO)) {
 					ct = fmttime(bp->ut_timefld, fulltime);
 					printf("%-*.*s  %-*.*s %-*.*s %s\n",
-					    namesz, namesz, bp->ut_name,
-					    linesz, linesz, bp->ut_line,
-					    hostsz, hostsz, bp->ut_host, ct);
+					    namesz, namesz, namebuf,
+					    linesz, linesz, linebuf,
+					    hostsz, hostsz, hostbuf, ct);
 					if (maxrec != -1 && !--maxrec)
 						return;
 				}
@@ -93,17 +132,14 @@
 			 * if the line is '{' or '|', date got set; see
 			 * utmp(5) for more info.
 			 */
-			if ((bp->ut_line[0] == '{' || bp->ut_line[0] == '|')
-			    && !bp->ut_line[1]) {
+			if ((linebuf[0] == '{' || linebuf[0] == '|')
+			    && !linebuf[1]) {
 				if (want(bp, NO)) {
 					ct = fmttime(bp->ut_timefld, fulltime);
 				printf("%-*.*s  %-*.*s %-*.*s %s\n",
-				    namesz, namesz,
-				    bp->ut_name,
-				    linesz, linesz,
-				    bp->ut_line,
-				    hostsz, hostsz,
-				    bp->ut_host,
+				    namesz, namesz, namebuf,
+				    linesz, linesz, linebuf,
+				    hostsz, hostsz, hostbuf,
 				    ct);
 					if (maxrec && !--maxrec)
 						return;
@@ -114,20 +150,20 @@
 			for (T = ttylist;; T = T->next) {
 				if (!T) {
 					/* add new one */
-					T = addtty(bp->ut_line);
+					T = addtty(linebuf);
 					break;
 				}
-				if (!strncmp(T->tty, bp->ut_line, LINESIZE))
+				if (!strncmp(T->tty, linebuf, LINESIZE))
 					break;
 			}
 			if (TYPE(bp) == SIGNATURE)
 				continue;
-			if (bp->ut_name[0] && want(bp, YES)) {
+			if (namebuf[0] && want(bp, YES)) {
 				ct = fmttime(bp->ut_timefld, fulltime);
 				printf("%-*.*s  %-*.*s %-*.*s %s ",
-				    namesz, namesz, bp->ut_name,
-				    linesz, linesz, bp->ut_line,
-				    hostsz, hostsz, bp->ut_host,
+				    namesz, namesz, namebuf,
+				    linesz, linesz, linebuf,
+				    hostsz, hostsz, hostbuf,
 				    ct);
 				if (!T->logout)
 					puts("  still logged in");
>Release-Note:
>Audit-Trail:
>Unformatted: