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);
front
cinap_lenrek 2014-11-07 12:51:59 +01:00
parent 5364fa720d
commit 797cc13c70
14 changed files with 32 additions and 33 deletions

View File

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

View File

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

View File

@ -196,14 +196,14 @@ timewait(int ms)
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

View File

@ -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;
}

View File

@ -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

View File

@ -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;
}

View File

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

View File

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

View File

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

View File

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

View File

@ -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;
}

View File

@ -1380,7 +1380,7 @@ initkbd(void)
if(e = recvp(c)){
chanfree(c);
c = nil;
werrstr(e);
werrstr("%s", e);
free(e);
}
return c;

View File

@ -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;
}

View File

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