shithub: riscv

Download patch

ref: 9e7ecc41d56148866725e26c872909823d515963
parent: 347ac6ef58d82e714358935568abcffd3509cfe8
author: cinap_lenrek <[email protected]>
date: Sun Sep 30 22:52:05 EDT 2012

devproc buffer overflow, strncpy

in devproc status read handler the p->status, p->text and p->user
could overflow the local statbuf buffer as they where copied into
it with code like: memmove(statbuf+someoff, p->text, strlen(p->text)).
now using readstr() which will truncate if the string is too long.

make strncpy() usage consistent, make sure results are always null
terminated.

--- a/sys/src/9/pc/devarch.c
+++ b/sys/src/9/pc/devarch.c
@@ -188,7 +188,7 @@
 	m->start = port;
 	m->end = port + size;
 	m->reserved = 1;
-	strncpy(m->tag, tag, sizeof(m->tag));
+	strncpy(m->tag, tag, sizeof(m->tag)-1);
 	m->tag[sizeof(m->tag)-1] = 0;
 	*l = m;
 
@@ -259,7 +259,7 @@
 	m->next = *l;
 	m->start = port;
 	m->end = port + size;
-	strncpy(m->tag, tag, sizeof(m->tag));
+	strncpy(m->tag, tag, sizeof(m->tag)-1);
 	m->tag[sizeof(m->tag)-1] = 0;
 	*l = m;
 
--- a/sys/src/9/pc/mouse.c
+++ b/sys/src/9/pc/mouse.c
@@ -321,6 +321,7 @@
 
 		mousetype = Mouseserial;
 		strncpy(mouseport, cb->f[1], sizeof(mouseport)-1);
+		mouseport[sizeof(mouseport)-1] = 0;
 		packetsize = 3;
 		break;
 	case CMhwaccel:
--- a/sys/src/9/pc/trap.c
+++ b/sys/src/9/pc/trap.c
@@ -159,7 +159,7 @@
 	v->tbdf = BUSUNKNOWN;
 	v->f = f;
 	v->a = a;
-	strncpy(v->name, name, KNAMELEN);
+	strncpy(v->name, name, KNAMELEN-1);
 	v->name[KNAMELEN-1] = 0;
 
 	ilock(&vctllock);
--- a/sys/src/9/pc/wavelan.c
+++ b/sys/src/9/pc/wavelan.c
@@ -1019,16 +1019,16 @@
 			p = cb->f[1];
 		if(ctlr->ptype == WPTypeAdHoc){
 			memset(ctlr->netname, 0, sizeof(ctlr->netname));
-			strncpy(ctlr->netname, p, WNameLen);
+			strncpy(ctlr->netname, p, WNameLen-1);
 		}
 		else{
 			memset(ctlr->wantname, 0, sizeof(ctlr->wantname));
-			strncpy(ctlr->wantname, p, WNameLen);
+			strncpy(ctlr->wantname, p, WNameLen-1);
 		}
 	}
 	else if(cistrcmp(cb->f[0], "station") == 0){
 		memset(ctlr->nodename, 0, sizeof(ctlr->nodename));
-		strncpy(ctlr->nodename, cb->f[1], WNameLen);
+		strncpy(ctlr->nodename, cb->f[1], WNameLen-1);
 	}
 	else if(cistrcmp(cb->f[0], "channel") == 0){
 		if((i = atoi(cb->f[1])) >= 1 && i <= 16)
--- a/sys/src/9/port/auth.c
+++ b/sys/src/9/port/auth.c
@@ -148,7 +148,7 @@
 
 	if(!iseve())
 		error(Eperm);
-	if(n >= DOMLEN)
+	if(n <= 0 || n >= DOMLEN)
 		error(Ebadarg);
 	memset(buf, 0, DOMLEN);
 	strncpy(buf, a, n);
--- a/sys/src/9/port/devaoe.c
+++ b/sys/src/9/port/devaoe.c
@@ -720,7 +720,8 @@
 
 	if((p = getconf("aoeif")) == nil)
 		return;
-	strncpy(ifbuf, p, sizeof buf);
+	strncpy(ifbuf, p, sizeof(ifbuf)-1);
+	ifbuf[sizeof(ifbuf)-1] = 0;
 	if((n = tokenize(ifbuf, f, nelem(f))) < 1)
 		return;
 	/* goo! */
@@ -1702,8 +1703,9 @@
 	nl->cc = cc;
 	nl->dc = dc;
 	nl->mtu = mtu;
-	strncpy(nl->path, path, sizeof nl->path);
-	memmove(nl->ea, ea, sizeof nl->ea);
+	strncpy(nl->path, path, sizeof(nl->path)-1);
+	nl->path[sizeof(nl->path)-1] = 0;
+	memmove(nl->ea, ea, sizeof(nl->ea));
 	poperror();
 	nl->flag |= Dup;
 	unlock(&netlinks);
--- a/sys/src/9/port/devbridge.c
+++ b/sys/src/9/port/devbridge.c
@@ -524,7 +524,7 @@
 		if(argc != 4)
 			error(usage);
 		type = Tether;
-		strncpy(name, argv[1], KNAMELEN);
+		strncpy(name, argv[1], KNAMELEN-1);
 		name[KNAMELEN-1] = 0;
 //		parseaddr(addr, argv[1], Eaddrlen);
 	} else if(strcmp(argv[0], "tunnel") == 0) {
@@ -531,7 +531,7 @@
 		if(argc != 5)
 			error(usage);
 		type = Ttun;
-		strncpy(name, argv[1], KNAMELEN);
+		strncpy(name, argv[1], KNAMELEN-1);
 		name[KNAMELEN-1] = 0;
 //		parseip(addr, argv[1]);
 		dev2 = argv[4];
@@ -632,12 +632,12 @@
 		error(usage);
 	if(strcmp(argv[0], "ether") == 0) {
 		type = Tether;
-		strncpy(name, argv[1], KNAMELEN);
+		strncpy(name, argv[1], KNAMELEN-1);
 		name[KNAMELEN-1] = 0;
 //		parseaddr(addr, argv[1], Eaddrlen);
 	} else if(strcmp(argv[0], "tunnel") == 0) {
 		type = Ttun;
-		strncpy(name, argv[1], KNAMELEN);
+		strncpy(name, argv[1], KNAMELEN-1);
 		name[KNAMELEN-1] = 0;
 //		parseip(addr, argv[1]);
 	} else
--- a/sys/src/9/port/devproc.c
+++ b/sys/src/9/port/devproc.c
@@ -796,7 +796,7 @@
 			m = strlen(p->note[0].msg) + 1;
 			if(m > n)
 				m = n;
-			memmove(va, p->note[0].msg, m);
+			memmove(va, p->note[0].msg, m-1);
 			((char*)va)[m-1] = '\0';
 			p->nnote--;
 			memmove(p->note, p->note+1, p->nnote*sizeof(Note));
@@ -850,12 +850,13 @@
 		sps = p->psstate;
 		if(sps == 0)
 			sps = statename[p->state];
+
 		memset(statbuf, ' ', sizeof statbuf);
-		memmove(statbuf+0*KNAMELEN, p->text, strlen(p->text));
-		memmove(statbuf+1*KNAMELEN, p->user, strlen(p->user));
-		memmove(statbuf+2*KNAMELEN, sps, strlen(sps));
-		j = 2*KNAMELEN + 12;
+		readstr(0, statbuf+0*KNAMELEN, KNAMELEN-1, p->text);
+		readstr(0, statbuf+1*KNAMELEN, KNAMELEN-1, p->user);
+		readstr(0, statbuf+2*KNAMELEN, 11, sps);
 
+		j = 2*KNAMELEN + 12;
 		for(i = 0; i < 6; i++) {
 			l = p->time[i];
 			if(i == TReal)
--- a/sys/src/9/port/devsdp.c
+++ b/sys/src/9/port/devsdp.c
@@ -811,7 +811,8 @@
 	c->ref = 2;
 	c->state = CInit;
 	c->in.window = ~0;
-	strncpy(c->owner, up->user, sizeof(c->owner));
+	strncpy(c->owner, up->user, sizeof(c->owner)-1);
+	c->owner[sizeof(c->owner)-1] = 0;
 	c->perm = 0660;
 	qunlock(c);
 
--- a/sys/src/9/port/devsegment.c
+++ b/sys/src/9/port/devsegment.c
@@ -533,7 +533,8 @@
 	for(done = 0; !done;){
 		sleep(&g->cmdwait, cmdready, g);
 		if(waserror()){
-			strncpy(g->err, up->errstr, sizeof(g->err));
+			strncpy(g->err, up->errstr, sizeof(g->err)-1);
+			g->err[sizeof(g->err)-1] = 0;
 		} else {
 			switch(g->cmd){
 			case Cstart:
--- a/sys/src/9/port/netif.c
+++ b/sys/src/9/port/netif.c
@@ -374,8 +374,10 @@
 		free(dir);
 		error(Eshortstat);
 	}
-	if(!emptystr(dir[0].uid))
-		strncpy(f->owner, dir[0].uid, KNAMELEN);
+	if(!emptystr(dir[0].uid)){
+		strncpy(f->owner, dir[0].uid, KNAMELEN-1);
+		f->owner[KNAMELEN-1] = 0;
+	}
 	if(dir[0].mode != ~0UL)
 		f->mode = dir[0].mode;
 	free(dir);
@@ -471,7 +473,8 @@
 			return -1;
 		}
 	}
-	strncpy(p->owner, o, KNAMELEN);
+	strncpy(p->owner, o, KNAMELEN-1);
+	p->owner[KNAMELEN-1] = 0;
 	p->mode = 0660;
 	unlock(&netlock);
 	return 0;
--- a/sys/src/9/port/qio.c
+++ b/sys/src/9/port/qio.c
@@ -1419,8 +1419,10 @@
 	q->state |= Qclosed;
 	if(msg == 0 || *msg == 0)
 		strcpy(q->err, Ehungup);
-	else
+	else {
 		strncpy(q->err, msg, ERRMAX-1);
+		q->err[ERRMAX-1] = 0;
+	}
 	iunlock(q);
 
 	/* wake up readers/writers */
--- a/sys/src/9/port/sysproc.c
+++ b/sys/src/9/port/sysproc.c
@@ -604,7 +604,7 @@
 		readnum(0, ow->time+TUser*NUMSIZE, NUMSIZE, w.time[TUser], NUMSIZE);
 		readnum(0, ow->time+TSys*NUMSIZE, NUMSIZE, w.time[TSys], NUMSIZE);
 		readnum(0, ow->time+TReal*NUMSIZE, NUMSIZE, w.time[TReal], NUMSIZE);
-		strncpy(ow->msg, w.msg, sizeof(ow->msg));
+		strncpy(ow->msg, w.msg, sizeof(ow->msg)-1);
 		ow->msg[sizeof(ow->msg)-1] = '\0';
 	}
 	return pid;