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);

diff --git a/sys/src/cmd/auth/factotum/util.c b/sys/src/cmd/auth/factotum/util.c
index 31f564e5b4b58ce076749d6c752dc2f5c0ef45cb..699a51bf513d67bf5ba9a13fdf6c5810722b240d 100644 (file)
--- a/sys/src/cmd/auth/factotum/util.c
+++ b/sys/src/cmd/auth/factotum/util.c
@@ -281,7 +281,7 @@ failure(Fsstate *s, char *fmt, ...)
                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;

diff --git a/sys/src/cmd/auth/passwd.c b/sys/src/cmd/auth/passwd.c
index cd9c98572409441f4bff247a1f924154914c834b..20369805f83731fb9b3cc1a13c78e4922695949c 100644 (file)
--- a/sys/src/cmd/auth/passwd.c
+++ b/sys/src/cmd/auth/passwd.c
@@ -29,7 +29,7 @@ asrdresp(int fd, char *buf, int len)
                        return -1;
                }
                error[AERRLEN-1] = 0;
-               werrstr(error);
+               errstr(error, sizeof error);
                return -1;
        default:
                werrstr(pbmsg);

diff --git a/sys/src/cmd/cec/cec.c b/sys/src/cmd/cec/cec.c
index f962a037e513e44c74c257f50350050103a93d22..90aae6217f512af71ecb49b669fb5e90698eb4c7 100644 (file)
--- a/sys/src/cmd/cec/cec.c
+++ b/sys/src/cmd/cec/cec.c
@@ -196,14 +196,14 @@ timewait(int ms)
 int
 didtimeout(void)
 {
-       char buf[ERRMAX];
-
-       rerrstr(buf, sizeof buf);
-       if(strcmp(buf, "interrupted") == 0){
-               werrstr(buf, 0);
-               return 1;
-       }
-       return 0;
+       char err[ERRMAX];
+       int rv;
+
+       *err = 0;
+       errstr(err, sizeof err);
+       rv = strcmp(err, "interrupted") == 0;
+       errstr(err, sizeof err);
+       return rv;
 }
 
 ushort

diff --git a/sys/src/cmd/cpu.c b/sys/src/cmd/cpu.c
index 361f568b20f891d67e859f2f5be40d49620b7492..1bfec1764ce1ec7dce556e1bf7d93ca00d694447 100644 (file)
--- a/sys/src/cmd/cpu.c
+++ b/sys/src/cmd/cpu.c
@@ -443,7 +443,7 @@ rexcall(int *fd, char *host, char *service)
                if(n < 0)
                        return "negotiating aan";
                if(*err){
-                       werrstr(err);
+                       errstr(err, sizeof err);
                        return negstr;
                }
        }
@@ -460,7 +460,7 @@ rexcall(int *fd, char *host, char *service)
        if(n < 0)
                return negstr;
        if(*err){
-               werrstr(err);
+               errstr(err, sizeof err);
                return negstr;
        }
 

diff --git a/sys/src/cmd/ip/ftpd.c b/sys/src/cmd/ip/ftpd.c
index 8626ab47e0a56104765ffe4e8f991b98a371ab33..1aa395b8906b76128577437cb4a13929e787e468 100644 (file)
--- a/sys/src/cmd/ip/ftpd.c
+++ b/sys/src/cmd/ip/ftpd.c
@@ -160,14 +160,11 @@ logit(char *fmt, ...)
 {
        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

diff --git a/sys/src/cmd/ip/ftpfs/proto.c b/sys/src/cmd/ip/ftpfs/proto.c
index 14fe56ee92e687bb7a28870b8a36eec632dfb9b7..cf4cb0f99acc71990b23e4db83744bce79788302 100644 (file)
--- a/sys/src/cmd/ip/ftpfs/proto.c
+++ b/sys/src/cmd/ip/ftpfs/proto.c
@@ -1324,7 +1324,7 @@ passive(int mode, Biobuf **bpp, char *cmda, char *cmdb)
                close(fd);
                if(debug)
                        fprint(2, "passive mode retrieve failed: %s\n", msg);
-               werrstr(msg);
+               werrstr("%s", msg);
                return x;
        }
 

diff --git a/sys/src/cmd/ip/tftpfs.c b/sys/src/cmd/ip/tftpfs.c
index 7f1fcb00353eaee1c9ea23ddb5bdc8f954368d1d..fa84d2b1de9cbc239d813d6f1b487affab774631 100644 (file)
--- a/sys/src/cmd/ip/tftpfs.c
+++ b/sys/src/cmd/ip/tftpfs.c
@@ -212,7 +212,7 @@ download(void *aux)
                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;
 

diff --git a/sys/src/cmd/ip/traceroute.c b/sys/src/cmd/ip/traceroute.c
index 33b6678b237a413ec904536dad5712332da6e778..3113d6dedc5a1e602fe4afb4875c98666ac3f9d0 100644 (file)
--- a/sys/src/cmd/ip/traceroute.c
+++ b/sys/src/cmd/ip/traceroute.c
@@ -140,8 +140,7 @@ static int
 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 @@ udpprobe(int cfd, int dfd, char *dest, int interval)
                        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 @@ static int
 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 @@ icmpprobe(int cfd, int dfd, char *dest, int interval)
                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 @@ main(int argc, char **argv)
        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 @@ main(int argc, char **argv)
                                done = 1;
                                continue;
                        }
+                       *err = 0;
                        errstr(err, sizeof err);
                        if(strstr(err, "refused")){
                                strcpy(hop, dest);

diff --git a/sys/src/cmd/jpg/readgif.c b/sys/src/cmd/jpg/readgif.c
index 0a5edd364a10a99948862d4bc2cfaeb0aef1b387..d5c0d1d7db24b3bb71a03046df9c7957239e265c 100644 (file)
--- a/sys/src/cmd/jpg/readgif.c
+++ b/sys/src/cmd/jpg/readgif.c
@@ -93,7 +93,7 @@ giferror(Header *h, char *fmt, ...)
        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);
 }

diff --git a/sys/src/cmd/jpg/readjpg.c b/sys/src/cmd/jpg/readjpg.c
index a3ab3ed8ac0c6750132c7f8b69ddc0553abd48c9..676ce2afb0cdad64653aaec7a6d38754a520cbff 100644 (file)
--- a/sys/src/cmd/jpg/readjpg.c
+++ b/sys/src/cmd/jpg/readjpg.c
@@ -227,7 +227,7 @@ jpgerror(Header *h, char *fmt, ...)
        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);
 }

diff --git a/sys/src/cmd/jpg/torgbv.c b/sys/src/cmd/jpg/torgbv.c
index ca84592b4150f7624530c885591cd5e62d793268..d1cef81a31e76227709b6c9a1f0844e49520b34c 100644 (file)
--- a/sys/src/cmd/jpg/torgbv.c
+++ b/sys/src/cmd/jpg/torgbv.c
@@ -16,13 +16,13 @@ void*
 _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;
 }
 

diff --git a/sys/src/cmd/rio/rio.c b/sys/src/cmd/rio/rio.c
index 5d7fe2e916218a8ba2b9f58dda5722e26d52b31b..052dcaff3757bb0217f4f8fa7b821b20886bf7c3 100644 (file)
--- a/sys/src/cmd/rio/rio.c
+++ b/sys/src/cmd/rio/rio.c
@@ -1380,7 +1380,7 @@ initkbd(void)
        if(e = recvp(c)){
                chanfree(c);
                c = nil;
-               werrstr(e);
+               werrstr("%s", e);
                free(e);
        }
        return c;

diff --git a/sys/src/cmd/wikifs/io.c b/sys/src/cmd/wikifs/io.c
index 43743744e9fb3203dbc3d7fb46582150dfe144cb..6be5f22c51dfdbb1bc76698f54d8b610ed205291 100644 (file)
--- a/sys/src/cmd/wikifs/io.c
+++ b/sys/src/cmd/wikifs/io.c
@@ -676,7 +676,7 @@ writepage(int num, ulong t, String *s, char *title)
        if(conflict){
                close(lfd);
                voidcache(num);
-               werrstr(err);
+               errstr(err, sizeof err);
                return -1;
        }
 

diff --git a/sys/src/libventi/client.c b/sys/src/libventi/client.c
index 40ee85175cb73d830cf53e64e0bdd4dba448ac19..80c03bf90105c36606848602cf482281730ef2d5 100644 (file)
--- a/sys/src/libventi/client.c
+++ b/sys/src/libventi/client.c
@@ -21,7 +21,7 @@ vtfcallrpc(VtConn *z, VtFcall *ou, VtFcall *in)
        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;