shithub: riscv

Download patch

ref: 797cc13c7053dbdd16c20dc4dee5aee8c92390b0
parent: 5364fa720de3b963a88dc4810ed83b4f2ab11d12
author: cinap_lenrek <[email protected]>
date: Fri Nov 7 07:51:59 EST 2014 

fix dangerous werrstr() usages

werrstr() takes a format string as its first argument.
a common error is to pass user controlled string buffers
into werrstr() that might contain format string escapes
causing werrstr() to take bogus arguments from the stack
and crash.

so instead of doing:
	werrstr(buf);

we want todo:
	werrstr("%s", buf);

or if we have a local ERRMAX sized buffer that we can override:
	errstr(buf, sizeof buf);


--- a/sys/src/cmd/auth/factotum/util.c
+++ b/sys/src/cmd/auth/factotum/util.c
@@ -281,7 +281,7 @@
 		vsnprint(e, sizeof e, fmt, arg);
 		va_end(arg);
 		strecpy(s->err, s->err+sizeof(s->err), e);
-		werrstr(e);
+		errstr(e, sizeof e);
 	}
 	flog("%d: failure %s", s->seqnum, s->err);
 	return RpcFailure;
--- a/sys/src/cmd/auth/passwd.c
+++ b/sys/src/cmd/auth/passwd.c
@@ -29,7 +29,7 @@
 			return -1;
 		}
 		error[AERRLEN-1] = 0;
-		werrstr(error);
+		errstr(error, sizeof error);
 		return -1;
 	default:
 		werrstr(pbmsg);
--- a/sys/src/cmd/cec/cec.c
+++ b/sys/src/cmd/cec/cec.c
@@ -196,14 +196,14 @@
 int
 didtimeout(void)
 {
-	char buf[ERRMAX];
+	char err[ERRMAX];
+	int rv;
 
-	rerrstr(buf, sizeof buf);
-	if(strcmp(buf, "interrupted") == 0){
-		werrstr(buf, 0);
-		return 1;
-	}
-	return 0;
+	*err = 0;
+	errstr(err, sizeof err);
+	rv = strcmp(err, "interrupted") == 0;
+	errstr(err, sizeof err);
+	return rv;
 }
 
 ushort
--- a/sys/src/cmd/cpu.c
+++ b/sys/src/cmd/cpu.c
@@ -443,7 +443,7 @@
 		if(n < 0)
 			return "negotiating aan";
 		if(*err){
-			werrstr(err);
+			errstr(err, sizeof err);
 			return negstr;
 		}
 	}
@@ -460,7 +460,7 @@
 	if(n < 0)
 		return negstr;
 	if(*err){
-		werrstr(err);
+		errstr(err, sizeof err);
 		return negstr;
 	}
 
--- a/sys/src/cmd/ip/ftpd.c
+++ b/sys/src/cmd/ip/ftpd.c
@@ -160,14 +160,11 @@
 {
 	char buf[8192];
 	va_list arg;
-	char errstr[ERRMAX];
 
-	rerrstr(errstr, sizeof errstr);
 	va_start(arg, fmt);
 	vseprint(buf, buf+sizeof(buf), fmt, arg);
 	va_end(arg);
 	syslog(0, FTPLOG, "%s.%s %s", nci->rsys, nci->rserv, buf);
-	werrstr(errstr, sizeof errstr);
 }
 
 static void
--- a/sys/src/cmd/ip/ftpfs/proto.c
+++ b/sys/src/cmd/ip/ftpfs/proto.c
@@ -1324,7 +1324,7 @@
 		close(fd);
 		if(debug)
 			fprint(2, "passive mode retrieve failed: %s\n", msg);
-		werrstr(msg);
+		werrstr("%s", msg);
 		return x;
 	}
 
--- a/sys/src/cmd/ip/tftpfs.c
+++ b/sys/src/cmd/ip/tftpfs.c
@@ -212,7 +212,7 @@
 		msg.buf[n] = 0;
 		switch(nhgets(msg.buf)){
 		case Tftp_ERROR:
-			werrstr((char*)msg.buf+4);
+			werrstr("%s", (char*)msg.buf+4);
 			err = "%r";
 			goto out;
 
--- a/sys/src/cmd/ip/traceroute.c
+++ b/sys/src/cmd/ip/traceroute.c
@@ -140,8 +140,7 @@
 udpprobe(int cfd, int dfd, char *dest, int interval)
 {
 	int n, i, rv;
-	char msg[Maxstring];
-	char err[Maxstring];
+	char msg[Maxstring], err[ERRMAX];
 
 	seek(cfd, 0, 0);
 	n = snprint(msg, sizeof msg, "connect %s", dest);
@@ -166,12 +165,13 @@
 			rv = 0;
 			break;
 		}
+		*err = 0;
 		errstr(err, sizeof err);
-		if(strstr(err, "alarm") == 0){
-			werrstr(err);
+		if(strcmp(err, "interrupted") != 0){
+			errstr(err, sizeof err);
 			break;
 		}
-		werrstr(err);
+		errstr(err, sizeof err);
 	}
 	alarm(0);
 	return rv;
@@ -185,7 +185,7 @@
 icmpprobe(int cfd, int dfd, char *dest, int interval)
 {
 	int x, i, n, len, rv;
-	char buf[512], err[Maxstring], msg[Maxstring];
+	char buf[512], err[ERRMAX], msg[Maxstring];
 	Icmphdr *ip;
 
 	seek(cfd, 0, 0);
@@ -212,12 +212,13 @@
 		n = read(dfd, buf, sizeof(buf));
 		alarm(0);
 		if(n < 0){
+			*err = 0;
 			errstr(err, sizeof err);
-			if(strstr(err, "alarm") == 0){
-				werrstr(err);
+			if(strcmp(err, "interrupted") != 0){
+				errstr(err, sizeof err);
 				break;
 			}
-			werrstr(err);
+			errstr(err, sizeof err);
 			continue;
 		}
 		x = (ip->seq[1]<<8) | ip->seq[0];
@@ -337,7 +338,7 @@
 	long *t;
 	char *net, *p;
 	char clone[Maxpath], dest[Maxstring], hop[Maxstring], dom[Maxstring];
-	char err[Maxstring];
+	char err[ERRMAX];
 	DS ds;
 
 	buckets = 0;
@@ -396,6 +397,7 @@
 				done = 1;
 				continue;
 			}
+			*err = 0;
 			errstr(err, sizeof err);
 			if(strstr(err, "refused")){
 				strcpy(hop, dest);
--- a/sys/src/cmd/jpg/readgif.c
+++ b/sys/src/cmd/jpg/readgif.c
@@ -93,7 +93,7 @@
 	vseprint(h->err, h->err+sizeof h->err, fmt, arg);
 	va_end(arg);
 
-	werrstr(h->err);
+	werrstr("%s", h->err);
 	giffreeall(h, 1);
 	longjmp(h->errlab, 1);
 }
--- a/sys/src/cmd/jpg/readjpg.c
+++ b/sys/src/cmd/jpg/readjpg.c
@@ -227,7 +227,7 @@
 	vseprint(h->err, h->err+sizeof h->err, fmt, arg);
 	va_end(arg);
 
-	werrstr(h->err);
+	werrstr("%s", h->err);
 	jpgfreeall(h, 1);
 	longjmp(h->errlab, 1);
 }
--- a/sys/src/cmd/jpg/torgbv.c
+++ b/sys/src/cmd/jpg/torgbv.c
@@ -16,13 +16,13 @@
 _remaperror(char *fmt, ...)
 {
 	va_list arg;
-	char buf[256];
+	char buf[ERRMAX];
 
 	va_start(arg, fmt);
 	vseprint(buf, buf+sizeof buf, fmt, arg);
 	va_end(arg);
 
-	werrstr(buf);
+	errstr(buf, sizeof buf);
 	return nil;
 }
 
--- a/sys/src/cmd/rio/rio.c
+++ b/sys/src/cmd/rio/rio.c
@@ -1380,7 +1380,7 @@
 	if(e = recvp(c)){
 		chanfree(c);
 		c = nil;
-		werrstr(e);
+		werrstr("%s", e);
 		free(e);
 	}
 	return c;
--- a/sys/src/cmd/wikifs/io.c
+++ b/sys/src/cmd/wikifs/io.c
@@ -676,7 +676,7 @@
 	if(conflict){
 		close(lfd);
 		voidcache(num);
-		werrstr(err);
+		errstr(err, sizeof err);
 		return -1;
 	}
 
--- a/sys/src/libventi/client.c
+++ b/sys/src/libventi/client.c
@@ -21,7 +21,7 @@
 	if(chattyventi)
 		fprint(2, "%s <- %F\n", argv0, in);
 	if(in->msgtype == VtRerror){
-		werrstr(in->error);
+		werrstr("%s", in->error);
 		vtfcallclear(in);
 		packetfree(p);
 		return -1;