maxmind/mmdbwriter

mismatched ip and mask size upon tree lookup results in invalid ipnet

oltdaniel opened this issue · 2 comments

NOTE: This is a issue that does not happen if the developer does not work with netip or manual constructions of net.IP that does not match the maximum size of an IP of the given tree options. More details below.

I'm currently playing around with this library and I ran into the issue of mismatched IP Mask sizes when using a mixed IPV4/IPv6 compatible tree. This targets the following line within this library:

mask := net.CIDRMask(prefixLen, t.treeDepth)
or
IP: ip.Mask(mask),

NOTE: The fix can be matching the size with the input ip, or matching the input ip with the mask size. More details below.

Example

Here is a minimal example code to produce the issue I had including "the correct way" for a lookup:

package main

import (
	"fmt"
	"log"
	"net"

	"github.com/maxmind/mmdbwriter"
	"github.com/maxmind/mmdbwriter/mmdbtype"
)

func main() {
	// Create a new writer
	writer, err := mmdbwriter.New(mmdbwriter.Options{DatabaseType: "Bug Report"})
	if err != nil {
		log.Fatal(err)
	}

	// IP we will use
	ip, network, _ := net.ParseCIDR("1.0.0.0/24")

	// Write a small sample
	err = writer.Insert(network, mmdbtype.Map{"country_code": mmdbtype.String("AU")})
	if err != nil {
		log.Fatal(err)
	}

	// "Incorrect" way for lookup
	{
		rNetwork, rData := writer.Get(net.IP{0x01, 0, 0, 0}) // This is a valid IP struct as referenced below
		fmt.Println(rNetwork, rData.(mmdbtype.Map)["country_code"]) // rNetwork should not print <nil>
	}

	// "Correct" way for lookup
	{
		rNetwork, rData := writer.Get(ip) // `net.IPv4(0x01, 0, 0, 0)` would also be possible as this is more explicit to represent a IPv4 with 16bytes
		fmt.Println(rNetwork, rData.(mmdbtype.Map)["country_code"])
	}
}

"Bug" in this library

The only issue is the manual construction of the *net.IPNet here:

return &net.IPNet{

Everything can be followed back to this specific sentence in the documentation for net.IP (here): "[...] accept either 4-byte (IPv4) or 16-byte (IPv6) slices as input. [...]". Internally they are not able to to automatically match the IP and Mask sizes, e.g. https://cs.opensource.google/go/go/+/refs/tags/go1.20.6:src/net/ip.go;l=497 , which results in an "incorrect" net.IPNet struct.

Thereby, matching the byte length of the input ip with the mask should fix this bug (or the other way around). A fix would be really helpful, as using netip is really useful to e.g. jump to the next ip. I'm also willing to submit a fix on my own if requested.

Thanks for the detailed report! I believe #65 will fix the issue.

Thanks for the detailed report! I believe #65 will fix the issue.

Thanks for the quick fix! Will test the code and give some feedback in the PR.