Subject: bin/17410: lam(1) contains numerous overflows
To: None <gnats-bugs@gnats.netbsd.org>
From: None <jmallett@FreeBSD.org>
List: netbsd-bugs
Date: 06/27/2002 07:10:01
>Number:         17410
>Category:       bin
>Synopsis:       lam(1) contains numerous overflows
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jun 27 07:10:01 PDT 2002
>Closed-Date:
>Last-Modified:
>Originator:     Juli Mallett
>Release:        -rHEAD
>Organization:
...
>Environment:
NA
>Description:
NetBSD's lam(1) contains numerous bugs and overflows which have been fixed in FreeBSD for some time.
>How-To-Repeat:
Something such as this is what tripped me:

lam `sed 's/.*/-/' resolve_conflicts.awk | xargs echo` < resolve_conflicts.awk
>Fix:
The diffs FreeBSD applied (using web form, apologies if formatting is lost; diffs between 1.4 and 1.5 of FreeBSD's lam.c will be correct, except for style differences [if any]):


@@ -50,6 +50,7 @@ static const char rcsid[] =
  *	Author:  John Kunze, UCB
  */
 
+#include <ctype.h>
 #include <err.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -82,7 +83,7 @@ main(argc, argv)
 	int argc;
 	char *argv[];
 {
-	register struct	openfile *ip;
+	struct	openfile *ip;
 
 	getargs(argv);
 	if (!morefiles)
@@ -104,9 +105,8 @@ void
 getargs(av)
 	char *av[];
 {
-	register struct	openfile *ip = input;
-	register char *p;
-	register char *c;
+	struct	openfile *ip = input;
+	char *p, *c;
 	static char fmtbuf[BUFSIZ];
 	char *fmtp = fmtbuf;
 	int P, S, F, T;
@@ -114,7 +114,8 @@ getargs(av)
 	P = S = F = T = 0;		/* capitalized options */
 	while ((p = *++av) != NULL) {
 		if (*p != '-' || !p[1]) {
-			morefiles++;
+			if (++morefiles >= MAXOFILES)
+				errx(1, "too many input files");
 			if (*p == '-')
 				ip->fp = stdin;
 			else if ((ip->fp = fopen(p, "r")) == NULL) {
@@ -130,7 +131,8 @@ getargs(av)
 			ip++;
 			continue;
 		}
-		switch (*(c = ++p) | 040) {
+		c = ++p;
+		switch (tolower(*c)) {
 		case 's':
 			if (*++p || (p = *++av))
 				ip->sepstring = p;
@@ -149,13 +151,19 @@ getargs(av)
 		case 'p':
 			ip->pad = 1;
 			P = (*c == 'P' ? 1 : 0);
+			/* FALLTHROUGH */
 		case 'f':
 			F = (*c == 'F' ? 1 : 0);
 			if (*++p || (p = *++av)) {
 				fmtp += strlen(fmtp) + 1;
-				if (fmtp > fmtbuf + BUFSIZ)
+				if (fmtp >= fmtbuf + sizeof(fmtbuf))
+					errx(1, "no more format space");
+				/* restrict format string to only valid width formatters */
+				if (strspn(p, "-.0123456789") != strlen(p))
+					errx(1, "invalid format string `%s'", p);
+				if (snprintf(fmtp, fmtbuf + sizeof(fmtbuf) - fmtp, "%%%ss", p)
+				    >= fmtbuf + sizeof(fmtbuf) - fmtp)
 					errx(1, "no more format space");
-				sprintf(fmtp, "%%%ss", p);
 				ip->format = fmtp;
 			}
 			else
@@ -175,13 +183,12 @@ char *
 pad(ip)
 	struct openfile *ip;
 {
-	register char *p = ip->sepstring;
-	register char *lp = linep;
+	char *lp = linep;
 
-	while (*p)
-		*lp++ = *p++;
+	strlcpy(lp, ip->sepstring, line + sizeof(line) - lp);
+	lp += strlen(lp);
 	if (ip->pad) {
-		sprintf(lp, ip->format, "");
+		snprintf(lp, line + sizeof(line) - lp, ip->format, "");
 		lp += strlen(lp);
 	}
 	return (lp);
@@ -192,10 +199,10 @@ gatherline(ip)
 	struct openfile *ip;
 {
 	char s[BUFSIZ];
-	register int c;
-	register char *p;
-	register char *lp = linep;
-	char *end = s + BUFSIZ;
+	int c;
+	char *p;
+	char *lp = linep;
+	char *end = s + sizeof(s) - 1;
 
 	if (ip->eof)
 		return (pad(ip));
@@ -210,10 +217,9 @@ gatherline(ip)
 		morefiles--;
 		return (pad(ip));
 	}
-	p = ip->sepstring;
-	while (*p)
-		*lp++ = *p++;
-	sprintf(lp, ip->format, s);
+	strlcpy(lp, ip->sepstring, line + sizeof(line) - lp);
+	lp += strlen(lp);
+	snprintf(lp, line + sizeof(line) - lp, ip->format, s);
 	lp += strlen(lp);
 	return (lp);
 }



>Release-Note:
>Audit-Trail:
>Unformatted: