pfsense/pfsense — pull request #958 by stilez (2014-02-21 01:14:21+00:00)
Title: Tighten and IPv6-ify gen_subnet() etc

Tightens, canonicalises and improves for IPv6, the functions
gen_subnet(), gen_subnetv6(), gen_subnet_max(), gen_subnetv6_max()

Changes are transparent to calling code.

Issues:

1) gen_subnet() and gen_subnet_max() will validate both IPv4 and IPv6 as valid args, but will then try to process an IPv6 subnet bitwise as x32 LONG without further checking, causing erroneous but apparently valid responses.

2) None of the functions properly sanitise their input: if $bits is >32 or >128, or even a non-integer, erroneous results will be passed back to the calling code as valid data without checking, again causing erroneous but apparently valid responses.

3) 3 of the 4 functions return an empty string for invalid but gen_subnetv6_max() returns a numeric value for invalid. Both responses loose-evaluate as False, but consistency is better.

Fixes and improvements:

1) The unspecified functions gen_subnet() and gen_subnet_max() now handle all args correctly, and don't mishandle if unexpectedly passed IPv6 or bad data.

2) Names are now canonical: gen_subnet(), gen_subnet_max() are now IPv4/v6 agnostic, and IPv4-only versions gen_subnetv4() and gen_subnetv4_max() are added as expected to exist, to match existing functions gen_subnetv6() and gen_subnetv6_max().

3) The return value for bad args is made consistent (empty string = False).

4) gen_subnetv6_max() now uses Net_IPv6's Ip2Bin() and Bin2Ip() functions and simple string manipulation rather than bitwise operations, so it's guaranteed 32-bit safe (compared to 128-bit bitwise operations in current code which seem less certain?)

5) Changes are transparent - the canonical functions still work exactly as before on IPv4 (only with proper bad arg validation) but also now work on IPv6 transparently, and on arbitrary IPv4/IPv6 data, similar to other functions like is_ipaddr().

Tested and handles valid but uncommon edge cases of /0, /32 (IPv4) and /128 (IPv6) correctly. Also avoids inet_ntop/pton if that's a real issue (previous PR comment had asked to avoid these functions)