Validate rule Advanced Options numeric entries

This makes sure the user puts in ordinary positive integers like "1" and "42" in these advanced options fields. It prevents everything else, including dodgy-looking possibilities like "007" which might actually work OK, but it is safer to allow just plain "7".
Note 1: The tests in function is_aoadv_used($rule_config) had to be changed back from using empty() to use $var != "" because if the user enters "0" in one of those fields and presses save, they get an error message, but the Advanced Options block on the GUI is closed (the "0" was considered empty()). That seemed rather confusing - the user would have had to click on the Advanced Options "Advanced" button again to open up that block and see the "0" they had entered.
Note 2: I have prohibited 2 things that "pf" allows into the ruleset without generating an error:
(max 0)
(tcp.established 0)
Both of these seem (IMHO) to have no valid use case. They would prevent states from ever happening, and so would effectively be block rules, which could be implemented easily as block rules.

1 files changed, 35 insertions, 7 deletions

diff --git a/usr/local/www/firewall_rules_edit.php b/usr/local/www/firewall_rules_edit.php
index 4d336cb..47bee3b 100755
--- a/usr/local/www/firewall_rules_edit.php
+++ b/usr/local/www/firewall_rules_edit.php
@@ -45,19 +45,25 @@ require("guiconfig.inc");
 require_once("filter.inc");
 require("shaper.inc");
 
+function is_posnumericint($arg) {
+	// Note that to be safe we do not allow any leading zero - "01", "007"
+	return (is_numericint($arg) && $arg[0] != '0' && $arg > 0);
+}
+
 function is_aoadv_used($rule_config) {
 	// Note that the user could set "tag" or "tagged" to the string "0", which is valid but empty().
+	// And if the user enters "0" in other fields, we want to present an error message, and keep the Advanced Options section open.
 	if ((isset($rule_config['allowopts'])) ||
 	    (isset($rule_config['disablereplyto'])) ||
 	    ($rule_config['tag'] != "") ||
 	    ($rule_config['tagged'] != "") ||
-	    (!empty($rule_config['max'])) ||
-	    (!empty($rule_config['max-src-nodes'])) ||
-	    (!empty($rule_config['max-src-conn'])) ||
-	    (!empty($rule_config['max-src-states'])) ||
-	    (!empty($rule_config['max-src-conn-rate'])) ||
-	    (!empty($rule_config['max-src-conn-rates'])) ||
-	    (!empty($rule_config['statetimeout'])))
+	    ($rule_config['max'] != "") ||
+	    ($rule_config['max-src-nodes'] != "") ||
+	    ($rule_config['max-src-conn'] != "") ||
+	    ($rule_config['max-src-states'] != "") ||
+	    ($rule_config['max-src-conn-rate'] != "") ||
+	    ($rule_config['max-src-conn-rates'] != "") ||
+	    ($rule_config['statetimeout'] != ""))
 		return true;
 	return false;
 }
@@ -531,6 +537,28 @@ if ($_POST) {
 			$input_errors[] = gettext("You cannot specify the state timeout (advanced option) if statetype is none and no L7 container is selected.");
 	}
 
+	if (($_POST['max'] != "") && !is_posnumericint($_POST['max']))
+		$input_errors[] = gettext("Maximum state entries (advanced option) must be a positive integer");
+
+	if (($_POST['max-src-nodes'] != "") && !is_posnumericint($_POST['max-src-nodes']))
+		$input_errors[] = gettext("Maximum number of unique source hosts (advanced option) must be a positive integer");
+
+	if (($_POST['max-src-conn'] != "") && !is_posnumericint($_POST['max-src-conn']))
+		$input_errors[] = gettext("Maximum number of established connections per host (advanced option) must be a positive integer");
+
+	if (($_POST['max-src-states'] != "") && !is_posnumericint($_POST['max-src-states']))
+		$input_errors[] = gettext("Maximum state entries per host (advanced option) must be a positive integer");
+
+	if (($_POST['max-src-conn-rate'] != "") && !is_posnumericint($_POST['max-src-conn-rate']))
+		$input_errors[] = gettext("Maximum new connections per host / per second(s) (advanced option) must be a positive integer");
+
+	if (($_POST['statetimeout'] != "") && !is_posnumericint($_POST['statetimeout']))
+		$input_errors[] = gettext("State timeout (advanced option) must be a positive integer");
+
+	if ((($_POST['max-src-conn-rate'] <> "" and $_POST['max-src-conn-rates'] == "")) || 
+	    (($_POST['max-src-conn-rate'] == "" and $_POST['max-src-conn-rates'] <> "")))
+		$input_errors[] = gettext("Both maximum new connections per host and the interval (per second(s)) must be specified");
+
 	if (!$_POST['tcpflags_any']) {
 		$settcpflags = array();
 		$outoftcpflags = array();