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)

1 files changed, 39 insertions, 27 deletions

diff --git a/etc/inc/util.inc b/etc/inc/util.inc
index 232dd30..94216a2 100644
--- a/etc/inc/util.inc
+++ b/etc/inc/util.inc
@@ -311,44 +311,56 @@ function is_numericint($arg) {
 	return (((is_int($arg) && $arg >= 0) || (is_string($arg) && strlen($arg) > 0 && ctype_digit($arg))) ? true : false);  
 }
 
-/* return the subnet address given a host address and a subnet bit count */
+/* Generate the (human readable) ipv4 or ipv6 subnet address (i.e., netmask, or subnet start IP) 
+   given an (human readable) ipv4 or ipv6 host address and subnet bit count */
 function gen_subnet($ipaddr, $bits) {
-	if (!is_ipaddr($ipaddr) || !is_numeric($bits))
-		return "";
-	return long2ip(ip2long($ipaddr) & gen_subnet_mask_long($bits));
+	if (($sn = gen_subnetv6($ipaddr, $bits)) == '')
+		$sn = gen_subnetv4($ipaddr, $bits);  // try to avoid rechecking IPv4/v6
+	return $sn;
+}
+								
+/* same as gen_subnet() but accepts IPv4 only */
+function gen_subnetv4($ipaddr, $bits) {
+	if (is_ipaddrv4($ipaddr) && is_numericint($bits) && $bits <= 32) {
+		if ($bits == 0)
+			return '0.0.0.0';  // avoids <<32
+		return long2ip(ip2long($ipaddr) & ((0xFFFFFFFF << (32 - $bits)) & 0xFFFFFFFF));
+	}
+	return "";
 }
 
-/* return the subnet address given a host address and a subnet bit count */
+/* same as gen_subnet() but accepts IPv6 only */
 function gen_subnetv6($ipaddr, $bits) {
-	if (!is_ipaddrv6($ipaddr) || !is_numeric($bits))
-		return "";
-
-	$address = Net_IPv6::getNetmask($ipaddr, $bits);
-	$address = Net_IPv6::compress($address);
-	return $address;
+	if (is_ipaddrv6($ipaddr) && is_numericint($bits) && $bits <= 128)
+		return Net_IPv6::compress(Net_IPv6::getNetmask($ipaddr, $bits));
+	return "";
 }
 
-/* return the highest (broadcast) address in the subnet given a host address and a subnet bit count */
+/* Generate the (human readable) ipv4 or ipv6 subnet end address (i.e., highest address, end IP, or IPv4 broadcast address) 
+   given an (human readable) ipv4 or ipv6 host address and subnet bit count. */
 function gen_subnet_max($ipaddr, $bits) {
-	if (!is_ipaddr($ipaddr) || !is_numeric($bits))
-		return "";
+	if (($sn = gen_subnetv6_max($ipaddr, $bits)) == '')
+		$sn = gen_subnetv4_max($ipaddr, $bits);  // try to avoid rechecking IPv4/v6
+	return $sn;
+}
 
-	return long2ip32(ip2long($ipaddr) | ~gen_subnet_mask_long($bits));
+/* same as gen_subnet_max() but validates IPv4 only */
+function gen_subnetv4_max($ipaddr, $bits) {
+	if (is_ipaddrv4($ipaddr) && is_numericint($bits) && $bits <= 32) {
+		if ($bits == 32)
+			return $ipaddr;
+		return long2ip(ip2long($ipaddr) | (0xFFFFFFFF >> $bits));
+	}
+	return "";
 }
 
-/* Generate end number for a given ipv6 subnet mask */
+/* same as gen_subnet_max() but validates IPv6 only */
 function gen_subnetv6_max($ipaddr, $bits) {
-	if(!is_ipaddrv6($ipaddr))
-		return false;
-
-	$mask = Net_IPv6::getNetmask('FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF',$bits);
-
-	$inet_ip = (binary)inet_pton($ipaddr);
-	$inet_mask = (binary)inet_pton($mask);
-
-	$inet_end = $inet_ip | ~$inet_mask;
-
-	return (inet_ntop($inet_end));
+	if (is_ipaddrv6($ipaddr) && is_numericint($bits) && $bits <= 128) {
+		$endip_bin = substr(Net_IPv6::_ip2Bin($ip), 0, $bits) . str_repeat('1', 128 - $bits);
+		return Net_IPv6::compress(Net_IPv6::_bin2Ip($endip_bin));
+	}
+	return "";
 }
 
 /* returns a subnet mask (long given a bit count) */