On Sat, Oct 30, 2010 at 07:26:00PM +0200, Peter J. Philipp wrote:

quoted text
> I had a look at the pack.c file where the DNS compression is being handled.
> It looks good to me.  But I have one concern that needs to be confirmed.
> In function dname_expand() on lines:
> 
>     54                          ptr = 256 * (n & ~0xc0) + data[offset + 1];
>     55                          if (ptr >= offset)
>     56                                  return (-1);
> 
> The pointer is checked against offset meaning that a compression loop can't 
> occur.  This is good.  However what happens if you have a DNS reply packet
> with a name with two labels in it, one being a normal label of a name and the 
> second being a compression pointer that points back to the first label, 
> kinda like so..
> 
> [8]centroid[C0 back to 8]
> 
> I'm worried it might go into an infinite loop or crash even.
> 
> Perhaps it should check that it cannot go back to a label inside a dns name that
> is being parsed.

You are totally right. The following patch should fix it. Thanks a lot.

Eric.

--- pack.c.orig	Sat Oct 30 21:22:14 2010
+++ pack.c	Sat Oct 30 21:22:06 2010
@@ -38,21 +38,21 @@ ssize_t
 dname_expand(const unsigned char *data, size_t len, size_t offset,
     size_t *newoffset, char *dst, size_t max)
 {
-	size_t		 n, count, end, ptr;
+	size_t		 n, count, end, ptr, start;
 	ssize_t		 res;
 
 	if (offset >= len)
 		return (-1);
 
 	res = 0;
-	end = offset;
+	end = start = offset;
 
 	for(; (n = data[offset]); ) {
 		if ((n & 0xc0) == 0xc0) {
 			if (offset + 2 > len)
 				return (-1);
 			ptr = 256 * (n & ~0xc0) + data[offset + 1];
-			if (ptr >= offset)
+			if (ptr >= start)
 				return (-1);
 			if (end < offset + 2)
 				end = offset + 2;
@@ -91,8 +91,9 @@ const char *
 dname_nthlabel(int n, const unsigned char *data, size_t len, size_t offset)
 {
 	int		 i;
-	size_t		 c, ptr;
+	size_t		 c, ptr, start;
 
+	start = offset;
 	for(i = 0;;) {
 		c = data[offset];
 		if (c == 0)
@@ -101,7 +102,7 @@ dname_nthlabel(int n, const unsigned char *data, size_
 			if (len < offset + 2)
 				return (NULL);
 			ptr = 256 * (c & ~0xc0) + data[offset + 1];
-			if (ptr >= offset)
+			if (ptr >= start)
 				return (NULL);
 			offset = ptr;
 			continue;
@@ -117,14 +118,15 @@ dname_nthlabel(int n, const unsigned char *data, size_
 ssize_t
 dname_count_labels(const unsigned char *data, size_t len, size_t offset)
 {
-	size_t c, n, ptr;
+	size_t c, n, ptr, start;
 
+	start = offset;
 	for(n = 0; (c = data[offset]); ) {
 		if ((c & 0xc0) == 0xc0) {
 			if (len < offset + 2)
 				return (-1);
 			ptr = 256 * (c & ~0xc0) + data[offset + 1];
-			if (ptr >= offset)
+			if (ptr >= start)
 				return (-1);
 			offset = ptr;
 			continue;