Add packet length parameter on readname, add tests

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;

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') {

read.c

@@ -15,14 +15,16 @@
  */
 
 #include <string.h>
+#include <stdio.h>
 
 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

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

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,26 +146,47 @@ 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");
 }