From aeae642fd87f22a0097507a24481139422cdf70d Mon Sep 17 00:00:00 2001 From: Erik Ekman Date: Mon, 6 Nov 2006 19:30:35 +0000 Subject: [PATCH] Add packet length parameter on readname, add tests --- dns.c | 10 ++++++---- iodined.c | 2 +- read.c | 22 +++++++++++++++++----- read.h | 2 +- test.c | 36 ++++++++++++++++++++++++++++++++---- 5 files changed, 57 insertions(+), 15 deletions(-) diff --git a/dns.c b/dns.c index 460932f..3a74e1f 100644 --- a/dns.c +++ b/dns.c @@ -309,12 +309,12 @@ dns_parse_reply(char *outbuf, int buflen, char *packet, int packetlen) rlen = 0; if(qdcount == 1) { - readname(packet, &data, name, sizeof(name)); + readname(packet, packetlen, &data, name, sizeof(name)); readshort(packet, &data, &type); readshort(packet, &data, &class); } if(ancount == 1) { - readname(packet, &data, name, sizeof(name)); + readname(packet, packetlen, &data, name, sizeof(name)); readshort(packet, &data, &type); readshort(packet, &data, &class); readlong(packet, &data, &ttl); @@ -455,7 +455,7 @@ dnsd_read(int fd, struct query *q, char *buf, int buflen) qdcount = ntohs(header->qdcount); if(qdcount == 1) { - readname(packet, &data, name, sizeof(name) -1); + readname(packet, r, &data, name, sizeof(name) -1); name[256] = 0; readshort(packet, &data, &type); readshort(packet, &data, &class); @@ -469,9 +469,11 @@ dnsd_read(int fd, struct query *q, char *buf, int buflen) rv = decodepacket(name, buf, buflen); } } - } else { + } else if (r < 0) { // Error perror("recvfrom"); rv = 0; + } else { // Packet too small to be dns protocol + rv = 0; } return rv; diff --git a/iodined.c b/iodined.c index bb7ed6f..b7bee46 100644 --- a/iodined.c +++ b/iodined.c @@ -111,7 +111,7 @@ tunnel(int tun_fd, int dns_fd) } if(FD_ISSET(dns_fd, &fds)) { read = dnsd_read(dns_fd, &q, in, sizeof(in)); - if (read < 0) + if (read <= 0) continue; if(in[0] == 'H' || in[0] == 'h') { diff --git a/read.c b/read.c index 07e3643..b2ff432 100644 --- a/read.c +++ b/read.c @@ -15,14 +15,16 @@ */ #include +#include static int -readname_loop(char *packet, char **src, char *dst, size_t length, size_t loop) +readname_loop(char *packet, int packetlen, char **src, char *dst, size_t length, size_t loop) { char *dummy; char *s; char *d; int len; + unsigned offset; char c; if (loop <= 0) @@ -36,8 +38,18 @@ readname_loop(char *packet, char **src, char *dst, size_t length, size_t loop) /* is this a compressed label? */ if((c & 0xc0) == 0xc0) { - dummy = packet + (((s[-1] & 0x3f) << 8) | s[0]); - len += readname_loop(packet, &dummy, d, length - len, loop - 1); + offset = (((s[-1] & 0x3f) << 8) | s[0]); + if (offset > packetlen) { + if (len == 0) { + // Bad jump first in packet + return 0; + } else { + // Bad jump after some data + break; + } + } + dummy = packet + offset; + len += readname_loop(packet, packetlen, &dummy, d, length - len, loop - 1); goto end; } @@ -65,9 +77,9 @@ end: } int -readname(char *packet, char **src, char *dst, size_t length) +readname(char *packet, int packetlen, char **src, char *dst, size_t length) { - return readname_loop(packet, src, dst, length, 10); + return readname_loop(packet, packetlen, src, dst, length, 10); } int diff --git a/read.h b/read.h index 6117b7b..27b0cbf 100644 --- a/read.h +++ b/read.h @@ -17,7 +17,7 @@ #ifndef _READ_H_ #define _READ_H_ -int readname(char *, char **, char *, size_t); +int readname(char *, int, char **, char *, size_t); int readshort(char *, char **, short *); int readlong(char *, char **, long *); int readdata(char *, char **, char *, size_t); diff --git a/test.c b/test.c index 19050df..d9545e2 100644 --- a/test.c +++ b/test.c @@ -129,6 +129,13 @@ test_readname() char onejump[] = "AA\x81\x80\x00\x01\x00\x00\x00\x00\x00\x00" "\x02hh\xc0\x15\x00\x01\x00\x01\x05zBCDE\x00"; + char badjump[] = { + 'A', 'A', 0x81, 0x80, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0xfe, 0xcc, 0x00, 0x01, 0x00, 0x01 }; + char badjump2[] = { + 'A', 'A', 0x81, 0x80, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x02, 'B', 'A', 0xfe, 0xcc, 0x00, 0x01, 0x00, 0x01 }; + char *jumper; char buf[1024]; char *data; int rv; @@ -139,25 +146,46 @@ test_readname() bzero(buf, sizeof(buf)); data = emptyloop + sizeof(HEADER); buf[1023] = 'A'; - rv = readname(emptyloop, &data, buf, 1023); + rv = readname(emptyloop, sizeof(emptyloop), &data, buf, 1023); assert(buf[1023] == 'A'); bzero(buf, sizeof(buf)); data = infloop + sizeof(HEADER); buf[4] = '\a'; - rv = readname(infloop, &data, buf, 4); + rv = readname(infloop, sizeof(infloop), &data, buf, 4); assert(buf[4] == '\a'); bzero(buf, sizeof(buf)); data = longname + sizeof(HEADER); buf[256] = '\a'; - rv = readname(longname, &data, buf, 256); + rv = readname(longname, sizeof(longname), &data, buf, 256); assert(buf[256] == '\a'); bzero(buf, sizeof(buf)); data = onejump + sizeof(HEADER); - rv = readname(onejump, &data, buf, 256); + rv = readname(onejump, sizeof(onejump), &data, buf, 256); assert(rv == 9); + + // These two tests use malloc to fail with segfault if jump is executed + bzero(buf, sizeof(buf)); + jumper = malloc(sizeof(badjump)); + if (jumper) { + memcpy(jumper, badjump, sizeof(badjump)); + data = jumper + sizeof(HEADER); + rv = readname(jumper, sizeof(badjump), &data, buf, 256); + assert(rv == 0); + } + free(jumper); + + bzero(buf, sizeof(buf)); + jumper = malloc(sizeof(badjump2)); + if (jumper) { + memcpy(jumper, badjump2, sizeof(badjump2)); + data = jumper + sizeof(HEADER); + rv = readname(jumper, sizeof(badjump2), &data, buf, 256); + assert(rv == 4); + } + free(jumper); printf("OK\n"); }