Merge pull request #3672 from phil-davis/handle-empty-port-alias-RELENG_2_3_3

6 files changed, 64 insertions, 27 deletions

diff --git a/src/etc/inc/filter.inc b/src/etc/inc/filter.inc
index 35e861f..48187e6 100644
--- a/src/etc/inc/filter.inc
+++ b/src/etc/inc/filter.inc
@@ -612,7 +612,20 @@ function filter_generate_scrubing() {
 	return $scrubrules;
 }
 
-function filter_generate_nested_alias($name, $alias, &$aliasnesting, &$aliasaddrnesting, &$use_filterdns = false) {
+function filter_generate_nested_alias($name) {
+	global $aliastable;
+
+	$aliasnesting = array();
+	$aliasaddrnesting = array();
+
+	if (($name == "") || !isset($aliastable[$name])) {
+		return "";
+	}
+
+	return filter_generate_nested_alias_recurse($name, $aliastable[$name], $aliasnesting, $aliasaddrnesting);
+}
+
+function filter_generate_nested_alias_recurse($name, $alias, &$aliasnesting, &$aliasaddrnesting, &$use_filterdns = false) {
 	global $aliastable, $filterdns;
 
 	$addresses = explode(" ", $alias);
@@ -629,7 +642,7 @@ function filter_generate_nested_alias($name, $alias, &$aliasnesting, &$aliasaddr
 		$tmpline = "";
 		if (is_alias($address)) {
 			if (alias_get_type($address) == 'urltable') {
-				// Feature#1603. For this type of alias we do not need to recursively call filter_generate_nested_alias. Just load IPs from the file.
+				// Feature#1603. For this type of alias we do not need to recursively call filter_generate_nested_alias_recurse. Just load IPs from the file.
 				$urltable_nesting = alias_expand_urltable($address);
 				if (!empty($urltable_nesting)) {
 					$urlfile_as_arr = file($urltable_nesting);
@@ -645,10 +658,10 @@ function filter_generate_nested_alias($name, $alias, &$aliasnesting, &$aliasaddr
 			}
 			/* We already expanded this alias so there is no necessity to do it again. */
 			else if (!isset($aliasnesting[$address])) {
-				$tmpline = filter_generate_nested_alias($name, $aliastable[$address], $aliasnesting, $aliasaddrnesting, $use_filterdns);
+				$tmpline = filter_generate_nested_alias_recurse($name, $aliastable[$address], $aliasnesting, $aliasaddrnesting, $use_filterdns);
 			}
 		} else if (!isset($aliasaddrnesting[$address])) {
-			if (!is_ipaddr($address) && !is_subnet($address) && !((($alias_type == 'port') || ($alias_type == 'url_ports')) && (is_port($address) || is_portrange($address))) && is_hostname($address)) {
+			if (!is_ipaddr($address) && !is_subnet($address) && !((($alias_type == 'port') || ($alias_type == 'url_ports')) && is_portorrange($address)) && is_hostname($address)) {
 				if (!isset($filterdns["{$address}{$name}"])) {
 					$use_filterdns = true;
 					$filterdns["{$address}{$name}"] = "pf {$address} {$name}\n";
@@ -689,9 +702,7 @@ function filter_expand_alias($alias_name) {
 	if (isset($config['aliases']['alias'])) {
 		foreach ($config['aliases']['alias'] as $aliased) {
 			if ($aliased['name'] == $alias_name) {
-				$aliasnesting = array();
-				$aliasaddrnesting = array();
-				return filter_generate_nested_alias($aliased['name'], $aliased['address'], $aliasnesting, $aliasaddrnesting);
+				return filter_generate_nested_alias($aliased['name']);
 			}
 		}
 	}
@@ -777,14 +788,12 @@ function filter_generate_aliases() {
 	/* Setup pf groups */
 	if (isset($config['aliases']['alias'])) {
 		foreach ($config['aliases']['alias'] as $aliased) {
-			$aliasnesting = array();
-			$aliasaddrnesting = array();
 			if (is_numericint($aliased['name'])) {
 				// skip aliases with numeric-only names. redmine #4289
 				file_notice("Filter_Reload", sprintf(gettext("Aliases with numeric-only names are not valid. Skipping alias %s"), $aliased['name']));
 				continue;
 			}
-			$addrlist = filter_generate_nested_alias($aliased['name'], $aliased['address'], $aliasnesting, $aliasaddrnesting);
+			$addrlist = filter_generate_nested_alias($aliased['name']);
 			switch ($aliased['type']) {
 			case "host":
 			case "network":
@@ -2728,18 +2737,36 @@ function filter_generate_user_rule($rule) {
 		return "# {$error_text}";
 	}
 	if ($rule['source']['port']
-	    && !(is_portrange(str_replace("-", ":", $rule['source']['port']))
-	    || alias_expand($rule['source']['port']))) {
-		$error_text = sprintf(gettext("Unresolvable source port alias '%1\$s' for rule '%2\$s'"), $rule['source']['port'], $rule['descr']);
-		file_notice("Filter_Reload", $error_text);
-		return "# {$error_text}";
+	    && !is_portorrange(str_replace("-", ":", $rule['source']['port']))) {
+		$error_text = "";
+
+		// It is not a literal port or port range, so alias should exist, and expand to something non-empty
+		if (!alias_expand($rule['source']['port'])) {
+			$error_text = sprintf(gettext("Unresolvable source port alias '%1\$s' for rule '%2\$s'"), $rule['source']['port'], $rule['descr']);
+		} else if (trim(filter_generate_nested_alias($rule['source']['port'])) == "") {
+			$error_text = sprintf(gettext("Empty source port alias '%1\$s' for rule '%2\$s'"), $rule['source']['port'], $rule['descr']);
+		}
+
+		if ($error_text) {
+			file_notice("Filter_Reload", $error_text);
+			return "# {$error_text}";
+		}
 	}
 	if ($rule['destination']['port']
-	    && !(is_portrange(str_replace("-", ":", $rule['destination']['port']))
-	    || alias_expand($rule['destination']['port']))) {
-		$error_text = sprintf(gettext("Unresolvable destination port alias '%1\$s' for rule '%2\$s'"), $rule['destination']['port'], $rule['descr']);
-		file_notice("Filter_Reload", $error_text);
-		return "# {$error_text}";
+	    && !is_portorrange(str_replace("-", ":", $rule['destination']['port']))) {
+		$error_text = "";
+
+		// It is not a literal port or port range, so alias should exist, and expand to something non-empty
+		if (!alias_expand($rule['destination']['port'])) {
+			$error_text = sprintf(gettext("Unresolvable destination port alias '%1\$s' for rule '%2\$s'"), $rule['destination']['port'], $rule['descr']);
+		} else if (trim(filter_generate_nested_alias($rule['destination']['port'])) == "") {
+			$error_text = sprintf(gettext("Empty destination port alias '%1\$s' for rule '%2\$s'"), $rule['destination']['port'], $rule['descr']);
+		}
+
+		if ($error_text) {
+			file_notice("Filter_Reload", $error_text);
+			return "# {$error_text}";
+		}
 	}
 	update_filter_reload_status(gettext("Setting up pass/block rules"));
 	$type = $rule['type'];
diff --git a/src/etc/inc/pfsense-utils.inc b/src/etc/inc/pfsense-utils.inc
index d822132..85d37d6 100644
--- a/src/etc/inc/pfsense-utils.inc
+++ b/src/etc/inc/pfsense-utils.inc
@@ -2241,7 +2241,7 @@ function parse_aliases_file($filename, $type = "url", $max_items = -1, $kflc = f
 				$tmp = $tmp_str;
 			}
 			$valid = (($type == "url" || $type == "urltable") && (is_ipaddr($tmp) || is_subnet($tmp))) ||
-				(($type == "url_ports" || $type == "urltable_ports") && (is_port($tmp) || is_portrange($tmp)));
+				(($type == "url_ports" || $type == "urltable_ports") && is_portorrange($tmp));
 			if ($valid) {
 				$items[] = $tmp;
 				if (count($items) == $max_items) {
diff --git a/src/etc/inc/util.inc b/src/etc/inc/util.inc
index aac095c..5b676f8 100644
--- a/src/etc/inc/util.inc
+++ b/src/etc/inc/util.inc
@@ -1161,6 +1161,11 @@ function is_portrange($portrange) {
 	return (count($ports) == 2 && is_port($ports[0]) && is_port($ports[1]));
 }
 
+/* returns true if $port is a valid TCP/UDP port number or range ("<port>:<port>") */
+function is_portorrange($port) {
+	return (is_port($port) || is_portrange($port));
+}
+
 /* returns true if $port is a valid port number or an alias thereof */
 function is_portoralias($port) {
 	global $config;
@@ -1179,6 +1184,11 @@ function is_portoralias($port) {
 	}
 }
 
+/* returns true if $port is a valid TCP/UDP port number or range ("<port>:<port>") or an alias thereof */
+function is_portorrangeoralias($port) {
+	return (is_portoralias($port) || is_portrange($port));
+}
+
 /* create ranges of sequential port numbers (200:215) and remove duplicates */
 function group_ports($ports, $kflc = false) {
 	if (!is_array($ports) || empty($ports)) {
@@ -1817,7 +1827,7 @@ function alias_expand($name) {
 			}
 		}
 		return "\${$name}";
-	} else if (is_ipaddr($name) || is_subnet($name) || is_port($name) || is_portrange($name)) {
+	} else if (is_ipaddr($name) || is_subnet($name) || is_portorrange($name)) {
 		return "{$name}";
 	} else {
 		return null;
diff --git a/src/usr/local/www/firewall_aliases_edit.php b/src/usr/local/www/firewall_aliases_edit.php
index bad45a5..5c8dfa6 100644
--- a/src/usr/local/www/firewall_aliases_edit.php
+++ b/src/usr/local/www/firewall_aliases_edit.php
@@ -452,7 +452,7 @@ if ($_POST) {
 					}
 				}
 			} else if ($_POST['type'] == "port") {
-				if (!is_port($input_address) && !is_portrange($input_address)) {
+				if (!is_portorrange($input_address)) {
 					$input_errors[] = sprintf(gettext("%s is not a valid port or alias."), $input_address);
 				}
 			} else if ($_POST['type'] == "host" || $_POST['type'] == "network") {
diff --git a/src/usr/local/www/firewall_aliases_import.php b/src/usr/local/www/firewall_aliases_import.php
index 8799dbb..83dd244 100644
--- a/src/usr/local/www/firewall_aliases_import.php
+++ b/src/usr/local/www/firewall_aliases_import.php
@@ -152,7 +152,7 @@ if ($_POST) {
 					if ($tab == "port") {
 						// Port alias
 						if (!empty($impip)) {
-							if (is_port($impip) || is_portrange($impip)) {
+							if (is_portorrange($impip)) {
 								$imported_ips[] = $impip;
 								$imported_descs[] = $impdesc;
 							} else {
diff --git a/src/usr/local/www/firewall_nat_out_edit.php b/src/usr/local/www/firewall_nat_out_edit.php
index 030ed7d..1fd0c3c 100644
--- a/src/usr/local/www/firewall_nat_out_edit.php
+++ b/src/usr/local/www/firewall_nat_out_edit.php
@@ -206,15 +206,15 @@ if ($_POST) {
 		$_POST['natport'] = trim($_POST['natport']);
 	}
 
-	if ($protocol_uses_ports && $_POST['sourceport'] <> "" && !(is_portoralias($_POST['sourceport']) || is_portrange($_POST['sourceport']))) {
+	if ($protocol_uses_ports && $_POST['sourceport'] <> "" && !is_portorrangeoralias($_POST['sourceport'])) {
 		$input_errors[] = gettext("A valid port or port alias must be supplied for the source port entry.");
 	}
 
-	if ($protocol_uses_ports && $_POST['dstport'] <> "" && !(is_portoralias($_POST['dstport']) || is_portrange($_POST['dstport']))) {
+	if ($protocol_uses_ports && $_POST['dstport'] <> "" && !is_portorrangeoralias($_POST['dstport'])) {
 		$input_errors[] = gettext("A valid port or port alias must be supplied for the destination port entry.");
 	}
 
-	if ($protocol_uses_ports && $_POST['natport'] <> "" && !(is_portoralias($_POST['natport']) || is_portrange($_POST['natport'])) && !isset($_POST['nonat'])) {
+	if ($protocol_uses_ports && $_POST['natport'] <> "" && !is_portorrangeoralias($_POST['natport']) && !isset($_POST['nonat'])) {
 		$input_errors[] = gettext("A valid port must be supplied for the NAT port entry.");
 	}