commit e9ce32ed9302b7d25bec918d30fc152f692d9134
parent 6e75d6c765c66b1db43664f797b4b8fbb2f6255b
Author: Kris Maglione <bsdaemon@wmii.de>
Date: Sun, 18 Jun 2006 21:23:17 -0400
Fixed buffer overflow vulnerabilities in ixp_unpack functions
Diffstat:
5 files changed, 136 insertions(+), 121 deletions(-)
diff --git a/cmd/wmiir.c b/cmd/wmiir.c
@@ -155,14 +155,14 @@ xls(void *result, unsigned int msize, int details)
static Stat stat;
do {
- ixp_unpack_stat(&p, &stat);
+ ixp_unpack_stat(&p, nil, &stat);
n++;
}
while(p - (unsigned char*)result < msize);
dir = (Stat *)cext_emallocz(sizeof(Stat) * n);
p = result;
do {
- ixp_unpack_stat(&p, &dir[i++]);
+ ixp_unpack_stat(&p, nil, &dir[i++]);
}
while(p - (unsigned char*)result < msize);
qsort(dir, n, sizeof(Stat), comp_stat);
@@ -189,7 +189,7 @@ xdir(char *file, int details)
return -1;
}
buf = c.fcall.stat;
- ixp_unpack_stat(&buf, s);
+ ixp_unpack_stat(&buf, nil, s);
if(!(s->mode & IXP_DMDIR)) {
print_stat(s, details);
fflush(stdout);
diff --git a/libixp/convert.c b/libixp/convert.c
@@ -12,36 +12,39 @@
void
ixp_pack_u8(unsigned char **msg, int *msize, unsigned char val)
{
- if((*msize -= 1) >= 0)
+ if(!msize || (*msize -= 1) >= 0)
*(*msg)++ = val;
}
void
-ixp_unpack_u8(unsigned char **msg, unsigned char *val)
+ixp_unpack_u8(unsigned char **msg, int *msize, unsigned char *val)
{
- *val = *(*msg)++;
+ if(!msize || (*msize -= 1) >= 0)
+ *val = *(*msg)++;
}
void
ixp_pack_u16(unsigned char **msg, int *msize, unsigned short val)
{
- if((*msize -= 2) >= 0) {
+ if(!msize || (*msize -= 2) >= 0) {
*(*msg)++ = val;
*(*msg)++ = val >> 8;
}
}
void
-ixp_unpack_u16(unsigned char **msg, unsigned short *val)
+ixp_unpack_u16(unsigned char **msg, int *msize, unsigned short *val)
{
- *val = *(*msg)++;
- *val |= *(*msg)++ << 8;
+ if(!msize || (*msize -= 2) >= 0) {
+ *val = *(*msg)++;
+ *val |= *(*msg)++ << 8;
+ }
}
void
ixp_pack_u32(unsigned char **msg, int *msize, unsigned int val)
{
- if((*msize -= 4) >= 0) {
+ if(!msize || (*msize -= 4) >= 0) {
*(*msg)++ = val;
*(*msg)++ = val >> 8;
*(*msg)++ = val >> 16;
@@ -50,18 +53,20 @@ ixp_pack_u32(unsigned char **msg, int *msize, unsigned int val)
}
void
-ixp_unpack_u32(unsigned char **msg, unsigned int *val)
+ixp_unpack_u32(unsigned char **msg, int *msize, unsigned int *val)
{
- *val = *(*msg)++;
- *val |= *(*msg)++ << 8;
- *val |= *(*msg)++ << 16;
- *val |= *(*msg)++ << 24;
+ if(!msize || (*msize -= 4) >= 0) {
+ *val = *(*msg)++;
+ *val |= *(*msg)++ << 8;
+ *val |= *(*msg)++ << 16;
+ *val |= *(*msg)++ << 24;
+ }
}
void
ixp_pack_u64(unsigned char **msg, int *msize, unsigned long long val)
{
- if((*msize -= 8) >= 0) {
+ if(!msize || (*msize -= 8) >= 0) {
*(*msg)++ = val;
*(*msg)++ = val >> 8;
*(*msg)++ = val >> 16;
@@ -74,12 +79,18 @@ ixp_pack_u64(unsigned char **msg, int *msize, unsigned long long val)
}
void
-ixp_unpack_u64(unsigned char **msg, unsigned long long *val)
+ixp_unpack_u64(unsigned char **msg, int *msize, unsigned long long *val)
{
- int i;
- *val = 0;
- for (i = 0; i < 8; ++i)
- *val |= (unsigned long long) *(*msg)++ << (8 * i);
+ if(!msize || (*msize -= 8) >= 0) {
+ *val |= *(*msg)++;
+ *val |= *(*msg)++ << 8;
+ *val |= *(*msg)++ << 16;
+ *val |= *(*msg)++ << 24;
+ *val |= (unsigned long long)*(*msg)++ << 32;
+ *val |= (unsigned long long)*(*msg)++ << 40;
+ *val |= (unsigned long long)*(*msg)++ << 48;
+ *val |= (unsigned long long)*(*msg)++ << 56;
+ }
}
void
@@ -92,13 +103,12 @@ ixp_pack_string(unsigned char **msg, int *msize, const char *s)
}
void
-ixp_unpack_strings(unsigned char **msg, unsigned short n, char **strings) {
+ixp_unpack_strings(unsigned char **msg, int *msize, unsigned short n, char **strings) {
unsigned char *s = *msg;
unsigned int i, size = 0;
unsigned short len;
- /* XXX: a specially crafted packet could make this read past the end of the buffer */
for(i=0; i<n; i++) {
- ixp_unpack_u16(&s, &len);
+ ixp_unpack_u16(&s, msize, &len);
s += len;
size += len + 1; /* for '\0' */
}
@@ -110,7 +120,10 @@ ixp_unpack_strings(unsigned char **msg, unsigned short n, char **strings) {
/* XXX: we don't really need mallocz here */
s = cext_emallocz(size);
for(i=0; i < n; i++) {
- ixp_unpack_u16(msg, &len);
+ ixp_unpack_u16(msg, msize, &len);
+ if(!msize || (*msize -= len) < 0)
+ return;
+
memcpy(s, *msg, len);
s[len] = '\0';
strings[i] = s;
@@ -120,55 +133,56 @@ ixp_unpack_strings(unsigned char **msg, unsigned short n, char **strings) {
}
void
-ixp_unpack_string(unsigned char **msg, char **string, unsigned short *len)
+ixp_unpack_string(unsigned char **msg, int *msize, char **string, unsigned short *len)
{
- ixp_unpack_u16(msg, len);
+ ixp_unpack_u16(msg, msize, len);
*string = nil;
- if (!*len)
- return;
- /* XXX we don't really need emallocz here */
- *string = cext_emallocz(*len+1);
- memcpy(*string, *msg, *len);
- (*string)[*len] = 0;
- *msg += *len;
+ if (*len && (!msize || (*msize -= *len) >= 0)) {
+ /* XXX we don't really need emallocz here */
+ *string = cext_emallocz(*len+1);
+ memcpy(*string, *msg, *len);
+ (*string)[*len] = 0;
+ *msg += *len;
+ }
}
void
ixp_pack_data(unsigned char **msg, int *msize, unsigned char *data, unsigned int datalen)
{
- if((*msize -= datalen) >= 0) {
+ if(!msize || (*msize -= datalen) >= 0) {
memcpy(*msg, data, datalen);
*msg += datalen;
}
}
void
-ixp_unpack_data(unsigned char **msg, unsigned char **data, unsigned int datalen)
+ixp_unpack_data(unsigned char **msg, int *msize, unsigned char **data, unsigned int datalen)
{
- /* XXX: this could be too large a number */
- *data = cext_emallocz(datalen);
- memcpy(*data, *msg, datalen);
- *msg += datalen;
+ if(!msize || (*msize -= datalen) >= 0) {
+ *data = cext_emallocz(datalen);
+ memcpy(*data, *msg, datalen);
+ *msg += datalen;
+ }
}
void
ixp_pack_prefix(unsigned char *msg, unsigned int size, unsigned char id,
unsigned short tag)
{
- int dummy = sizeof(unsigned char) +
- sizeof(unsigned short) + sizeof(unsigned int);
- ixp_pack_u32(&msg, &dummy, size);
- ixp_pack_u8(&msg, &dummy, id);
- ixp_pack_u16(&msg, &dummy, tag);
+ ixp_pack_u32(&msg, 0, size);
+ ixp_pack_u8(&msg, 0, id);
+ ixp_pack_u16(&msg, 0, tag);
}
void
ixp_unpack_prefix(unsigned char **msg, unsigned int *size, unsigned char *id,
unsigned short *tag)
{
- ixp_unpack_u32(msg, size);
- ixp_unpack_u8(msg, id);
- ixp_unpack_u16(msg, tag);
+ int msize;
+ ixp_unpack_u32(msg, nil, size);
+ msize = *size;
+ ixp_unpack_u8(msg, &msize, id);
+ ixp_unpack_u16(msg, &msize, tag);
}
void
@@ -180,11 +194,11 @@ ixp_pack_qid(unsigned char **msg, int *msize, Qid * qid)
}
void
-ixp_unpack_qid(unsigned char **msg, Qid * qid)
+ixp_unpack_qid(unsigned char **msg, int *msize, Qid * qid)
{
- ixp_unpack_u8(msg, &qid->type);
- ixp_unpack_u32(msg, &qid->version);
- ixp_unpack_u64(msg, &qid->path);
+ ixp_unpack_u8(msg, msize, &qid->type);
+ ixp_unpack_u32(msg, msize, &qid->version);
+ ixp_unpack_u64(msg, msize, &qid->path);
}
void
@@ -205,19 +219,19 @@ ixp_pack_stat(unsigned char **msg, int *msize, Stat * stat)
}
void
-ixp_unpack_stat(unsigned char **msg, Stat * stat)
+ixp_unpack_stat(unsigned char **msg, int *msize, Stat * stat)
{
unsigned short dummy;
*msg += sizeof(unsigned short);
- ixp_unpack_u16(msg, &stat->type);
- ixp_unpack_u32(msg, &stat->dev);
- ixp_unpack_qid(msg, &stat->qid);
- ixp_unpack_u32(msg, &stat->mode);
- ixp_unpack_u32(msg, &stat->atime);
- ixp_unpack_u32(msg, &stat->mtime);
- ixp_unpack_u64(msg, &stat->length);
- ixp_unpack_string(msg, &stat->name, &dummy);
- ixp_unpack_string(msg, &stat->uid, &dummy);
- ixp_unpack_string(msg, &stat->gid, &dummy);
- ixp_unpack_string(msg, &stat->muid, &dummy);
+ ixp_unpack_u16(msg, msize, &stat->type);
+ ixp_unpack_u32(msg, msize, &stat->dev);
+ ixp_unpack_qid(msg, msize, &stat->qid);
+ ixp_unpack_u32(msg, msize, &stat->mode);
+ ixp_unpack_u32(msg, msize, &stat->atime);
+ ixp_unpack_u32(msg, msize, &stat->mtime);
+ ixp_unpack_u64(msg, msize, &stat->length);
+ ixp_unpack_string(msg, msize, &stat->name, &dummy);
+ ixp_unpack_string(msg, msize, &stat->uid, &dummy);
+ ixp_unpack_string(msg, msize, &stat->gid, &dummy);
+ ixp_unpack_string(msg, msize, &stat->muid, &dummy);
}
diff --git a/libixp/ixp.h b/libixp/ixp.h
@@ -321,28 +321,28 @@ int ixp_client_do_fcall(IXPClient * c);
/* convert.c */
void ixp_pack_u8(unsigned char **msg, int *msize, unsigned char val);
-void ixp_unpack_u8(unsigned char **msg, unsigned char *val);
+void ixp_unpack_u8(unsigned char **msg, int *msize, unsigned char *val);
void ixp_pack_u16(unsigned char **msg, int *msize, unsigned short val);
-void ixp_unpack_u16(unsigned char **msg, unsigned short *val);
+void ixp_unpack_u16(unsigned char **msg, int *msize, unsigned short *val);
void ixp_pack_u32(unsigned char **msg, int *msize, unsigned int val);
-void ixp_unpack_u32(unsigned char **msg, unsigned int *val);
+void ixp_unpack_u32(unsigned char **msg, int *msize, unsigned int *val);
void ixp_pack_u64(unsigned char **msg, int *msize, unsigned long long val);
-void ixp_unpack_u64(unsigned char **msg, unsigned long long *val);
+void ixp_unpack_u64(unsigned char **msg, int *msize, unsigned long long *val);
void ixp_pack_string(unsigned char **msg, int *msize, const char *s);
-void ixp_unpack_strings(unsigned char **msg, unsigned short n, char **strings);
-void ixp_unpack_string(unsigned char **msg, char **string, unsigned short *len);
+void ixp_unpack_strings(unsigned char **msg, int *msize, unsigned short n, char **strings);
+void ixp_unpack_string(unsigned char **msg, int *msize, char **string, unsigned short *len);
void ixp_pack_data(unsigned char **msg, int *msize, unsigned char *data,
unsigned int datalen);
-void ixp_unpack_data(unsigned char **msg, unsigned char **data,
+void ixp_unpack_data(unsigned char **msg, int *msize, unsigned char **data,
unsigned int datalen);
void ixp_pack_prefix(unsigned char *msg, unsigned int size,
unsigned char id, unsigned short tag);
void ixp_unpack_prefix(unsigned char **msg, unsigned int *size,
unsigned char *id, unsigned short *tag);
void ixp_pack_qid(unsigned char **msg, int *msize, Qid *qid);
-void ixp_unpack_qid(unsigned char **msg, Qid *qid);
+void ixp_unpack_qid(unsigned char **msg, int *msize, Qid *qid);
void ixp_pack_stat(unsigned char **msg, int *msize, Stat *stat);
-void ixp_unpack_stat(unsigned char **msg, Stat *stat);
+void ixp_unpack_stat(unsigned char **msg, int *msize, Stat *stat);
/* request.c */
void respond(Req *r, char *error);
diff --git a/libixp/message.c b/libixp/message.c
@@ -139,103 +139,104 @@ ixp_fcall2msg(void *msg, Fcall *fcall, unsigned int msglen)
unsigned int
ixp_msg2fcall(Fcall *fcall, void *msg, unsigned int msglen)
{
- unsigned int i, msize;
+ unsigned int i, msize, tsize;
unsigned short len;
unsigned char *p = msg;
ixp_unpack_prefix(&p, &msize, &fcall->type, &fcall->tag);
+ tsize = msize;
if(msize > msglen) /* bad message */
return 0;
switch (fcall->type) {
case TVERSION:
case RVERSION:
- ixp_unpack_u32(&p, &fcall->msize);
- ixp_unpack_string(&p, &fcall->version, &len);
+ ixp_unpack_u32(&p, &msize, &fcall->msize);
+ ixp_unpack_string(&p, &msize, &fcall->version, &len);
break;
case TAUTH:
- ixp_unpack_u32(&p, &fcall->afid);
- ixp_unpack_string(&p, &fcall->uname, &len);
- ixp_unpack_string(&p, &fcall->aname, &len);
+ ixp_unpack_u32(&p, &msize, &fcall->afid);
+ ixp_unpack_string(&p, &msize, &fcall->uname, &len);
+ ixp_unpack_string(&p, &msize, &fcall->aname, &len);
break;
case RAUTH:
- ixp_unpack_qid(&p, &fcall->aqid);
+ ixp_unpack_qid(&p, &msize, &fcall->aqid);
break;
case RATTACH:
- ixp_unpack_qid(&p, &fcall->qid);
+ ixp_unpack_qid(&p, &msize, &fcall->qid);
break;
case TATTACH:
- ixp_unpack_u32(&p, &fcall->fid);
- ixp_unpack_u32(&p, &fcall->afid);
- ixp_unpack_string(&p, &fcall->uname, &len);
- ixp_unpack_string(&p, &fcall->aname, &len);
+ ixp_unpack_u32(&p, &msize, &fcall->fid);
+ ixp_unpack_u32(&p, &msize, &fcall->afid);
+ ixp_unpack_string(&p, &msize, &fcall->uname, &len);
+ ixp_unpack_string(&p, &msize, &fcall->aname, &len);
break;
case RERROR:
- ixp_unpack_string(&p, &fcall->ename, &len);
+ ixp_unpack_string(&p, &msize, &fcall->ename, &len);
break;
case TFLUSH:
- ixp_unpack_u16(&p, &fcall->oldtag);
+ ixp_unpack_u16(&p, &msize, &fcall->oldtag);
break;
case TWALK:
- ixp_unpack_u32(&p, &fcall->fid);
- ixp_unpack_u32(&p, &fcall->newfid);
- ixp_unpack_u16(&p, &fcall->nwname);
- ixp_unpack_strings(&p, fcall->nwname, fcall->wname);
+ ixp_unpack_u32(&p, &msize, &fcall->fid);
+ ixp_unpack_u32(&p, &msize, &fcall->newfid);
+ ixp_unpack_u16(&p, &msize, &fcall->nwname);
+ ixp_unpack_strings(&p, &msize, fcall->nwname, fcall->wname);
break;
case RWALK:
- ixp_unpack_u16(&p, &fcall->nwqid);
+ ixp_unpack_u16(&p, &msize, &fcall->nwqid);
for(i = 0; i < fcall->nwqid; i++)
- ixp_unpack_qid(&p, &fcall->wqid[i]);
+ ixp_unpack_qid(&p, &msize, &fcall->wqid[i]);
break;
case TOPEN:
- ixp_unpack_u32(&p, &fcall->fid);
- ixp_unpack_u8(&p, &fcall->mode);
+ ixp_unpack_u32(&p, &msize, &fcall->fid);
+ ixp_unpack_u8(&p, &msize, &fcall->mode);
break;
case ROPEN:
case RCREATE:
- ixp_unpack_qid(&p, &fcall->qid);
- ixp_unpack_u32(&p, &fcall->iounit);
+ ixp_unpack_qid(&p, &msize, &fcall->qid);
+ ixp_unpack_u32(&p, &msize, &fcall->iounit);
break;
case TCREATE:
- ixp_unpack_u32(&p, &fcall->fid);
- ixp_unpack_string(&p, &fcall->name, &len);
- ixp_unpack_u32(&p, &fcall->perm);
- ixp_unpack_u8(&p, &fcall->mode);
+ ixp_unpack_u32(&p, &msize, &fcall->fid);
+ ixp_unpack_string(&p, &msize, &fcall->name, &len);
+ ixp_unpack_u32(&p, &msize, &fcall->perm);
+ ixp_unpack_u8(&p, &msize, &fcall->mode);
break;
case TREAD:
- ixp_unpack_u32(&p, &fcall->fid);
- ixp_unpack_u64(&p, &fcall->offset);
- ixp_unpack_u32(&p, &fcall->count);
+ ixp_unpack_u32(&p, &msize, &fcall->fid);
+ ixp_unpack_u64(&p, &msize, &fcall->offset);
+ ixp_unpack_u32(&p, &msize, &fcall->count);
break;
case RREAD:
- ixp_unpack_u32(&p, &fcall->count);
- ixp_unpack_data(&p, &fcall->data, fcall->count);
+ ixp_unpack_u32(&p, &msize, &fcall->count);
+ ixp_unpack_data(&p, &msize, &fcall->data, fcall->count);
break;
case TWRITE:
- ixp_unpack_u32(&p, &fcall->fid);
- ixp_unpack_u64(&p, &fcall->offset);
- ixp_unpack_u32(&p, &fcall->count);
- ixp_unpack_data(&p, &fcall->data, fcall->count);
+ ixp_unpack_u32(&p, &msize, &fcall->fid);
+ ixp_unpack_u64(&p, &msize, &fcall->offset);
+ ixp_unpack_u32(&p, &msize, &fcall->count);
+ ixp_unpack_data(&p, &msize, &fcall->data, fcall->count);
break;
case RWRITE:
- ixp_unpack_u32(&p, &fcall->count);
+ ixp_unpack_u32(&p, &msize, &fcall->count);
break;
case TCLUNK:
case TREMOVE:
case TSTAT:
- ixp_unpack_u32(&p, &fcall->fid);
+ ixp_unpack_u32(&p, &msize, &fcall->fid);
break;
case RSTAT:
- ixp_unpack_u16(&p, &len);
- ixp_unpack_data(&p, &fcall->stat, len);
+ ixp_unpack_u16(&p, &msize, &len);
+ ixp_unpack_data(&p, &msize, &fcall->stat, len);
break;
case TWSTAT:
- ixp_unpack_u32(&p, &fcall->fid);
- ixp_unpack_u16(&p, &len);
- ixp_unpack_data(&p, &fcall->stat, len);
+ ixp_unpack_u32(&p, &msize, &fcall->fid);
+ ixp_unpack_u16(&p, &msize, &len);
+ ixp_unpack_data(&p, &msize, &fcall->stat, len);
break;
}
- if(msg + msize == p)
- return msize;
+ if(msize > 0)
+ return tsize;
return 0;
}
diff --git a/libixp/transport.c b/libixp/transport.c
@@ -64,7 +64,7 @@ ixp_recv_message(int fd, void *msg, unsigned int msglen, char **errstr)
if(ixp_recv_data(fd, msg, sizeof(unsigned int), errstr) !=
sizeof(unsigned int))
return 0;
- ixp_unpack_u32((void *)&msg, &msize);
+ ixp_unpack_u32((void *)&msg, nil, &msize);
if(msize > msglen) {
*errstr = "invalid message header";
return 0;
