git » spf » commit 541dee7

diff --git a/spf.go b/spf.go
index 06a90db..3de4bf3 100644
--- a/spf.go
+++ b/spf.go
@@ -73,6 +73,9 @@ var qualToResult = map[byte]Result{
 // situations may change over time, and new ones may be added. Be careful
 // about over-relying on these.
 var (
+	// Errors related to the arguments given to the check functions.
+	ErrInvalidIPAddress = errors.New("invalid IP address")
+
 	// Errors related to an invalid SPF record.
 	ErrUnknownField  = errors.New("unknown field")
 	ErrInvalidIP     = errors.New("invalid ipX value")
@@ -132,6 +135,8 @@ type Option func(*resolution)
 // the check as per RFC, as well as an error for debugging purposes. Note that
 // the error may be non-nil even on successful checks.
 //
+// If `ip` is not a valid IP address, the result is PermError.
+//
 // Reference: https://tools.ietf.org/html/rfc7208#section-4
 //
 // Deprecated: use CheckHostWithSender instead.
@@ -146,9 +151,24 @@ func CheckHost(ip net.IP, domain string) (Result, error) {
 		resolver:     defaultResolver,
 		trace:        defaultTrace,
 	}
+
+	if !validIP(ip) {
+		r.trace("invalid ip %v", ip)
+		return PermError, ErrInvalidIPAddress
+	}
+
 	return r.Check(domain)
 }
 
+// validIP checks that the given IP address is valid. Because net.IP is a byte
+// slice, it can hold values that are not valid addresses, including nil
+// (which is what net.ParseIP returns on error).
+func validIP(ip net.IP) bool {
+	// To16 returns nil unless the address is a valid IP address (4 or 16
+	// bytes long).
+	return ip.To16() != nil
+}
+
 // CheckHostWithSender fetches SPF records for `sender`'s domain, parses them,
 // and evaluates them to determine if `ip` is permitted to send mail for it.
 // The `helo` domain is used if the sender has no domain part.
@@ -160,6 +180,8 @@ func CheckHost(ip net.IP, domain string) (Result, error) {
 // the check as per RFC, as well as an error for debugging purposes. Note that
 // the error may be non-nil even on successful checks.
 //
+// If `ip` is not a valid IP address, the result is PermError.
+//
 // Reference: https://tools.ietf.org/html/rfc7208#section-4
 func CheckHostWithSender(ip net.IP, helo, sender string, opts ...Option) (Result, error) {
 	_, domain := split(sender)
@@ -182,6 +204,11 @@ func CheckHostWithSender(ip net.IP, helo, sender string, opts ...Option) (Result
 		opt(r)
 	}
 
+	if !validIP(ip) {
+		r.trace("invalid ip %v", ip)
+		return PermError, ErrInvalidIPAddress
+	}
+
 	return r.Check(domain)
 }
 
@@ -1059,6 +1086,10 @@ func ipToMacroStr(ip net.IP) string {
 	for _, b := range ip.To16() {
 		fmt.Fprintf(&sb, "%x.%x.", b>>4, b&0xf)
 	}
+
 	// Return the string without the trailing ".".
-	return sb.String()[:sb.Len()-1]
+	// Note that on an invalid address To16 returns nil, and the loop above
+	// writes nothing; the entry points reject those, but we take care not to
+	// assume the string is non-empty anyway.
+	return strings.TrimSuffix(sb.String(), ".")
 }
diff --git a/spf_test.go b/spf_test.go
index b860ffe..caa76bc 100644
--- a/spf_test.go
+++ b/spf_test.go
@@ -535,6 +535,64 @@ func TestIPMatchHelper(t *testing.T) {
 	}
 }
 
+func TestInvalidIP(t *testing.T) {
+	dns := NewDefaultResolver()
+	defaultTrace = t.Logf
+
+	// A record that would match anything, to make sure we are rejecting the
+	// address and not just failing to match it.
+	dns.Txt["domain"] = []string{"v=spf1 exists:%{i}.d1111 +all"}
+	dns.Ip["d1111"] = []net.IP{ip1111}
+
+	// net.IP is a byte slice, so it can hold things that are not valid
+	// addresses. net.ParseIP returns nil on error, which is the most likely
+	// one to show up in practice.
+	cases := []net.IP{
+		nil,
+		net.ParseIP("this is not an ip"),
+		{},
+		{1, 2, 3},
+		{1, 2, 3, 4, 5},
+	}
+
+	for _, ip := range cases {
+		res, err := CheckHost(ip, "domain")
+		if res != PermError || err != ErrInvalidIPAddress {
+			t.Errorf("CheckHost(%v): expected %v/%v, got %v/%v",
+				[]byte(ip), PermError, ErrInvalidIPAddress, res, err)
+		}
+
+		res, err = CheckHostWithSender(ip, "helo", "user@domain")
+		if res != PermError || err != ErrInvalidIPAddress {
+			t.Errorf("CheckHostWithSender(%v): expected %v/%v, got %v/%v",
+				[]byte(ip), PermError, ErrInvalidIPAddress, res, err)
+		}
+	}
+}
+
+func TestIPToMacroStr(t *testing.T) {
+	cases := []struct {
+		ip  net.IP
+		out string
+	}{
+		{ip1111, "1.1.1.1"},
+		{net.ParseIP("::ffff:1.1.1.1"), "1.1.1.1"},
+		{ip6666, "2.0.0.1.0.d.b.8.0.0.0.0.0.0.0.0." +
+			"0.0.0.0.0.0.0.0.0.0.0.0.0.0.6.8"},
+
+		// Invalid addresses: the entry points reject them, but the helper
+		// must not panic on them either.
+		{nil, ""},
+		{net.IP{1, 2, 3}, ""},
+	}
+	for _, c := range cases {
+		if out := ipToMacroStr(c.ip); out != c.out {
+			t.Errorf("ipToMacroStr(%v): expected %q, got %q",
+				[]byte(c.ip), c.out, out)
+		}
+	}
+}
+
 func TestInvalidMacro(t *testing.T) {
 	// Test that the macro expansion detects some invalid macros.
 	macros := []string{