Project

General

Profile

« Previous | Next » 

Revision a5e2a35f

Added by Stilez y over 11 years ago

Tighten is_subnet() functions

The is_subnet(), is_subnetv4() and is_subnetv6() functions have significant issues in their coding logic.

Issues:

1) Functions use is_numeric(), so they validate invalid bitcount parts such as '1.1.1.1/6.5' or '::8000/94.7' as valid subnet strings

2) Validation explodes to a list of 2 variables rather than testing the whole string, so it's possible to craft a malicious string or mis-enter text such as "::8000/24/;\nARBITRARYTEXT;", or any valid subnet followed by a third '/' character followed by arbitrary text, which always validates as permitted data. In v2.x this allows arbitrary or malformed text placement into core config files such as pf.conf, f/w tables, and probably exec() strings, from the GUI (reported previously)

3) is_subnet() and is_subnetv6() return false for the valid subnet /0 as a CIDR, although technically this is allowed and valid as a subnet (it may have unambiguous and useful meaning in a router or security context).

The existing code is easier to rewrite than fix. It's a lot shorter too. A single preg_match() does all the separating of valid subnet parts and identifies which if either type of subnet the string could be, by exactly matching EITHER (7-15 char string containing only 0-9.) OR (2-39 char string containing only 0-9a-f:) followed by '/' and (1-3 numeric digits). Depending which of the first two matches is non-empty, it then checks the "ip" part" and "numeric" part are each valid, and exits. Efficient and robust.

NOTE: - these functions returned a True/false value ( is_subnet() or !is_subnet() ). As any positive number evaluates in a Boolean sense to True, I have taken advantage of this to have "success" return 4 or 6 (not just "true") to say the type of subnet matched. It's no extra effort or cost, and turns out to allow a lot of efficiency and code simplification elsewhere, since calling code that needs to validate a subnet and test if it's IPv4 or IPv6, a common situation, can now do this reliably with a single is_subnet() call:

if (($sn = is_subnet($data)) {
if ($sn == 4)
IPv4 handler code...
else
IPv6 handler code...
} else
error handler code...

I've used it here to write is_subnetv4() and is_subnetv6() to be single line near-aliases of is_subnet(), that just check the return value is indeed 4 or 6.

View differences:

etc/inc/util.inc
593 593

  
594 594
}
595 595

  
596
/* returns true if $subnet is a valid IPv4 or IPv6 subnet in CIDR format */
596
/* returns true if $subnet is a valid IPv4 or IPv6 subnet in CIDR format
597
   	false - if not a valid subnet
598
   	true (numeric 4 or 6) - if valid, gives type of subnet */
597 599
function is_subnet($subnet) {
598
	if(is_subnetv4($subnet)) {
599
		return true;
600
	}
601
	if(is_subnetv6($subnet)) {
602
		return true;
600
	if (is_string($subnet) && preg_match('/^(?:([0-9.]{7,15})|([0-9a-f:]{2,39}))\/(\d{1,3})$/i', $subnet, $parts)) {
601
		if (is_ipaddrv4($parts[1]) && $parts[3] <= 32)
602
			return 4;
603
		if (is_ipaddrv6($parts[2]) && $parts[3] <= 128)
604
			return 6;
603 605
	}
604 606
	return false;
605 607
}
606 608

  
607
/* returns true if $subnet is a valid IPv4 subnet in CIDR format */
609
/* same as is_subnet() but accepts IPv4 only */
608 610
function is_subnetv4($subnet) {
609
	if (!is_string($subnet))
610
		return false;
611

  
612
	list($hp,$np) = explode('/', $subnet);
613

  
614
	if (!is_ipaddrv4($hp))
615
		return false;
616

  
617
	if (!is_numeric($np) || ($np < 1) || ($np > 32))
618
		return false;
619

  
620
	return true;
611
	return (is_subnet($subnet) == 4);
621 612
}
622 613

  
623
/* returns true if $subnet is a valid IPv6 subnet in CIDR format */
614
/* same as is_subnet() but accepts IPv6 only */
624 615
function is_subnetv6($subnet) {
625
	if (!is_string($subnet))
626
		return false;
627

  
628
	list($hp,$np) = explode('/', $subnet);
629

  
630
	if (!is_ipaddrv6($hp))
631
		return false;
632

  
633
	if (!is_numeric($np) || ($np < 1) || ($np > 128))
634
		return false;
635

  
636
	return true;
616
	return (is_subnet($subnet) == 6);
637 617
}
638 618

  
639

  
640 619
/* returns true if $subnet is a valid subnet in CIDR format or an alias thereof */
641 620
function is_subnetoralias($subnet) {
642 621
	global $aliastable;

Also available in: Unified diff