massimocandela / geofeed-finder

Utility to find geofeed files linked from rpsl.
BSD 3-Clause "New" or "Revised" License
74 stars 8 forks source link

Possible issue with v6 prefix parsing/matching #12

Closed ggidofalvy-tc closed 2 years ago

ggidofalvy-tc commented 2 years ago

Hi there,

I seem to have found a small issue with v6 prefix lookups, I'm not actually sure if this is an issue with our geofeed itself or the code.

$ # Supernet / inetnum containing geofeed entry
$ ./geofeed-finder-linux-x64 -t 2606:8e80::/32
inetnum: null https://geoip.tingfiber.net/tf-geofeed.csv [cache]
Cannot read property 'toLowerCase' of null
$ # Exact match
$ ./geofeed-finder-linux-x64 -t 2606:8e80:800::/37
inetnum: null https://geoip.tingfiber.net/tf-geofeed.csv [cache]
Cannot read property 'toLowerCase' of null
$ # Subnet match
$ ./geofeed-finder-linux-x64 -t 2606:8e80:800::/38
inetnum: 2606:8e80:0800:0000:0000:0000:0000:0000/37 https://geoip.tingfiber.net/tf-geofeed.csv [cache]
2606:8e80:800::/37,US,US-CA,Solana Beach,

I'd think that the latter two should be a match, while the first one should just return gracefully instead of trying to call functions on a null, resulting in an error.

I have also found another error where a shorter IPv6 prefix is somehow matched, resulting in all v6 entries in a geofeed being returned:

$ ./geofeed-finder-linux-x64 -t 2607:fb90:10::/44
inetnum: 2607:fb90:0000:0000:0000:0000:0000:0000/28 https://raw.githubusercontent.com/tmobile/tmus-geofeed/main/tmus-geo-ip.txt [cache]
2607:fb90:c13e:b::/64,US,US-WA,Seattle,
2607:fb90:c13f:fffd::/64,US,US-TX,Dallas,
2607:FB90::/28,US,,,
2607:fb90:10::/44,US,US-MI,Detroit,
2607:fb90:1200::/44,US,US-FL,Orlando,
2607:fb90:1210::/44,US,US-FL,Orlando,
2607:fb90:1220::/44,US,US-FL,Orlando,
...
massimocandela commented 2 years ago

Hi @ggidofalvy-tc,

The flag -t must be used only for testing the resolution of the geofeed file. When you test a prefix, you obtain the "parent" inetnum + the content of the geofeed file for that inetnum. So the behavior reported in the the second example is ok. However, it's poorly documented, I will address this.

For your first report, the inetnum should not be printed as "null", I will investigate on this. Just to clarify, the normal parsing (without -t) is not affected.

massimocandela commented 2 years ago

Hi @ggidofalvy-tc,

I fixed the issue and improved the documentation about the -t option (see v1.5.1). Thanks a lot for reporting this.