Fix "too many compression points" for valid message (#835)

* Increase the maximum number of allowed compression pointers

* Add a Pack+Unpack test case for many compression pointers

* Clarify maxCompressionPointers comment

msg.go

@@ -24,6 +24,18 @@ import (
 const (
 	maxCompressionOffset    = 2 << 13 // We have 14 bits for the compression pointer
 	maxDomainNameWireOctets = 255     // See RFC 1035 section 2.3.4
+
+	// This is the maximum number of compression pointers that should occur in a
+	// semantically valid message. Each label in a domain name must be at least one
+	// octet and is separated by a period. The root label won't be represented by a
+	// compression pointer to a compression pointer, hence the -2 to exclude the
+	// smallest valid root label.
+	//
+	// It is possible to construct a valid message that has more compression pointers
+	// than this, and still doesn't loop, by pointing to a previous pointer. This is
+	// not something a well written implementation should ever do, so we leave them
+	// to trip the maximum compression pointer check.
+	maxCompressionPointers = (maxDomainNameWireOctets+1)/2 - 2
 )
 
 // Errors defined in this package.
@@ -399,7 +411,7 @@ Loop:
 			if ptr == 0 {
 				off1 = off
 			}
-			if ptr++; ptr > 10 {
+			if ptr++; ptr > maxCompressionPointers {
 				return "", lenmsg, &Error{err: "too many compression pointers"}
 			}
 			// pointer should guarantee that it advances and points forwards at least

msg_test.go

@@ -257,3 +257,23 @@ func TestPackDomainNameNSECTypeBitmap(t *testing.T) {
 		t.Logf("got:      %v", msg2.Answer[1])
 	}
 }
+
+func TestPackUnpackManyCompressionPointers(t *testing.T) {
+	m := new(Msg)
+	m.Compress = true
+	m.SetQuestion("example.org.", TypeNS)
+
+	for domain := "a."; len(domain) < maxDomainNameWireOctets; domain += "a." {
+		m.Answer = append(m.Answer, &NS{Hdr: RR_Header{Name: domain, Rrtype: TypeNS, Class: ClassINET}, Ns: "example.org."})
+
+		b, err := m.Pack()
+		if err != nil {
+			t.Fatalf("Pack failed for %q and %d records with: %v", domain, len(m.Answer), err)
+		}
+
+		var m2 Msg
+		if err := m2.Unpack(b); err != nil {
+			t.Fatalf("Unpack failed for %q and %d records with: %v", domain, len(m.Answer), err)
+		}
+	}
+}