apparentlymart/go-cidr

BUG: AddressRange returns wrong value

peske opened this issue · 1 comments

peske commented

Expected behavior

If we have net.IPNet which contains 100.100.0.0/16, cidr.AddressRange function should always return 100.100.0.0 and 100.100.255.255

Actual behavior

The method sometimes returns 100.100.0.0 and 0:ffff:ffff:ffff:ffff:ffff:ffff:ffff

Steps to reproduce

func main() {
	srcNet := net.IPNet{
		IP:   net.IP{100, 100, 0, 0},
		Mask: net.IPMask{255, 255, 0, 0},
	}
	fmt.Printf("Source net: %v\n", srcNet)
	fmt.Printf("Source IP length: %d\n", len(srcNet.IP))
	fmt.Printf("Source mask length: %d\n", len(srcNet.Mask))
	from, to := cidr.AddressRange(&srcNet)
	fmt.Printf("Source range: %v - %v\n", from, to)
	serialized, err := json.Marshal(&srcNet)
	if err != nil {
		log.Fatalf("Failed to serialize the network: %v\n", err)
	}
	dstNet := net.IPNet{}
	err = json.Unmarshal(serialized, &dstNet)
	if err != nil {
		log.Fatalf("Failed to deserialize the network: %v\n", err)
	}
	fmt.Printf("Dest net: %v\n", dstNet)
	fmt.Printf("Dest IP length: %d\n", len(dstNet.IP))
	fmt.Printf("Dest mask length: %d\n", len(dstNet.Mask))
	from, to = cidr.AddressRange(&dstNet)
	fmt.Printf("Dest range: %v - %v\n", from, to)
}

produces the following output:

Source net: {100.100.0.0 ffff0000}
Source IP length: 4
Source mask length: 4
Source range: 100.100.0.0 - 100.100.255.255
Dest net: {100.100.0.0 ffff0000}
Dest IP length: 16
Dest mask length: 4
Dest range: 100.100.0.0 - 0:ffff:ffff:ffff:ffff:ffff:ffff:ffff

Cause

The problem arises because you're relying on len(ip) to determine if ip is v4 or v6, which is not a reliable way because IPv4 can be represented in 16 bytes (IPv4 as IPv6). For example, 100.100.0.0 is represented in 16 bytes in the following way: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 100, 100, 0, 0], and in this case len(ip) will return 16, although it's obviously IPv4. In short: instead of using len(ip) == 4 to test if it's an IPv4 address, you should use net.To4(ip) != nil (len(net.To4(ip)) in this example will be equal to 4).

But how IPv4 of length 16 happens in the first place? There are many ways but in this particular case, it happened because json.Unmarshal always deserializes IPs to length 16, no matter if it's IPv4 or IPv6.

The bug from this example is in ipToInt function, but it may happen that you're using len(ip) check in some other places in your code.

tdbs commented

+1