From 12884c91a67c21b7a5bfaf2acec14335b6a7e738 Mon Sep 17 00:00:00 2001 From: Pascal Date: Fri, 3 May 2019 22:32:57 +0200 Subject: [PATCH 01/16] #564 fix #564 by allowing CIDR in mysql host validation. fix english and german field description accordingly --- lib/Froxlor/Validate/Check.php | 2 +- lng/english.lng.php | 2 +- lng/german.lng.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Froxlor/Validate/Check.php b/lib/Froxlor/Validate/Check.php index 41901d31..147afb5a 100644 --- a/lib/Froxlor/Validate/Check.php +++ b/lib/Froxlor/Validate/Check.php @@ -78,7 +78,7 @@ class Check foreach ($mysql_access_host_array as $host_entry) { - if (Validate::validate_ip2($host_entry, true, 'invalidip', true, true) == false && Validate::validateDomain($host_entry) == false && Validate::validateLocalHostname($host_entry) == false && $host_entry != '%') { + if (Validate::validate_ip2($host_entry, true, 'invalidip', true, true, true) == false && Validate::validateDomain($host_entry) == false && Validate::validateLocalHostname($host_entry) == false && $host_entry != '%') { return array( self::FORMFIELDS_PLAUSIBILITY_CHECK_ERROR, 'invalidmysqlhost', diff --git a/lng/english.lng.php b/lng/english.lng.php index 45bced52..f6e342ec 100644 --- a/lng/english.lng.php +++ b/lng/english.lng.php @@ -569,7 +569,7 @@ $lng['serversettings']['apacheconf_htpasswddir']['description'] = 'Where should $lng['error']['formtokencompromised'] = 'The request seems to be compromised. For security reasons you were logged out.'; $lng['serversettings']['mysql_access_host']['title'] = 'MySQL-Access-Hosts'; -$lng['serversettings']['mysql_access_host']['description'] = 'A comma separated list of hosts from which users should be allowed to connect to the MySQL-Server.'; +$lng['serversettings']['mysql_access_host']['description'] = 'A comma separated list of hosts from which users should be allowed to connect to the MySQL-Server. To allow a subnet, CIDR syntax like 100.127.0.0/255.255.0.0 or 10.10.10.10/16 is valid.'; // ADDED IN 1.2.18-svn1 diff --git a/lng/german.lng.php b/lng/german.lng.php index 551ff001..d57fa638 100644 --- a/lng/german.lng.php +++ b/lng/german.lng.php @@ -564,7 +564,7 @@ $lng['serversettings']['apacheconf_htpasswddir']['description'] = 'Wo sollen die $lng['error']['formtokencompromised'] = 'Das Formular scheint manipuliert worden zu sein. Aus Sicherheitsgründen wurden Sie ausgelogged.'; $lng['serversettings']['mysql_access_host']['title'] = 'MySQL-Access-Hosts'; -$lng['serversettings']['mysql_access_host']['description'] = 'Eine durch Komma getrennte Liste mit den Hostnamen aller Hostnames/IP-Adressen, von denen sich die Benutzer einloggen dürfen.'; +$lng['serversettings']['mysql_access_host']['description'] = 'Eine durch Komma getrennte Liste mit den Hostnamen aller Hostnames/IP-Adressen, von denen sich die Benutzer einloggen dürfen. Um ein Subnetz zu erlauben, die CIDR Schreibweise (Beispiel 100.127.0.0/255.255.0.0 oder 10.10.10.10/16) ist erlaubt.'; // ADDED IN 1.2.18-svn1 From 0f4d8d76ae76b5edc4d15ba641ef2ded174138e1 Mon Sep 17 00:00:00 2001 From: Pascal Date: Fri, 3 May 2019 23:31:31 +0200 Subject: [PATCH 02/16] #564 fix wording --- lng/english.lng.php | 2 +- lng/german.lng.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lng/english.lng.php b/lng/english.lng.php index f6e342ec..672df707 100644 --- a/lng/english.lng.php +++ b/lng/english.lng.php @@ -569,7 +569,7 @@ $lng['serversettings']['apacheconf_htpasswddir']['description'] = 'Where should $lng['error']['formtokencompromised'] = 'The request seems to be compromised. For security reasons you were logged out.'; $lng['serversettings']['mysql_access_host']['title'] = 'MySQL-Access-Hosts'; -$lng['serversettings']['mysql_access_host']['description'] = 'A comma separated list of hosts from which users should be allowed to connect to the MySQL-Server. To allow a subnet, CIDR syntax like 100.127.0.0/255.255.0.0 or 10.10.10.10/16 is valid.'; +$lng['serversettings']['mysql_access_host']['description'] = 'A comma separated list of hosts from which users should be allowed to connect to the MySQL-Server. To allow a subnet, long CIDR syntax like 100.127.0.0/255.255.0.0 is valid.'; // ADDED IN 1.2.18-svn1 diff --git a/lng/german.lng.php b/lng/german.lng.php index d57fa638..bcb57709 100644 --- a/lng/german.lng.php +++ b/lng/german.lng.php @@ -564,7 +564,7 @@ $lng['serversettings']['apacheconf_htpasswddir']['description'] = 'Wo sollen die $lng['error']['formtokencompromised'] = 'Das Formular scheint manipuliert worden zu sein. Aus Sicherheitsgründen wurden Sie ausgelogged.'; $lng['serversettings']['mysql_access_host']['title'] = 'MySQL-Access-Hosts'; -$lng['serversettings']['mysql_access_host']['description'] = 'Eine durch Komma getrennte Liste mit den Hostnamen aller Hostnames/IP-Adressen, von denen sich die Benutzer einloggen dürfen. Um ein Subnetz zu erlauben, die CIDR Schreibweise (Beispiel 100.127.0.0/255.255.0.0 oder 10.10.10.10/16) ist erlaubt.'; +$lng['serversettings']['mysql_access_host']['description'] = 'Eine durch Komma getrennte Liste mit den Hostnamen aller Hostnames/IP-Adressen, von denen sich die Benutzer einloggen dürfen. Um ein Subnetz zu erlauben, die lange CIDR Schreibweise (Beispiel 100.127.0.0/255.255.0.0) ist erlaubt.'; // ADDED IN 1.2.18-svn1 From f297058461c99391837d4759eda20d50c661ed90 Mon Sep 17 00:00:00 2001 From: Pascal Date: Sat, 4 May 2019 00:39:12 +0200 Subject: [PATCH 03/16] #564 fix wording add validation for cidr syntax add automatic transform of cidr to netmask for mysql --- lib/Froxlor/Settings/Store.php | 22 +++++++++++++++++++++ lib/Froxlor/Validate/Validate.php | 33 ++++++++++++++++++++++++++----- lng/english.lng.php | 2 +- lng/german.lng.php | 2 +- 4 files changed, 52 insertions(+), 7 deletions(-) diff --git a/lib/Froxlor/Settings/Store.php b/lib/Froxlor/Settings/Store.php index 5c567d31..9e3627f6 100644 --- a/lib/Froxlor/Settings/Store.php +++ b/lib/Froxlor/Settings/Store.php @@ -290,6 +290,28 @@ class Store public static function storeSettingMysqlAccessHost($fieldname, $fielddata, $newfieldvalue) { + $ips = $newfieldvalue; + //Convert cidr to netmask for mysql, if needed be + if(strpos($ips, ',') !== false) { + $ips = explode(',', $ips); + } + if(is_array($ips) && count($ips) > 0) { + $newfieldvalue = []; + foreach ($ips as $ip) { + $org_ip = $ip; + $ip_cidr = explode("/", $ip); + if (count($ip_cidr) == 2) { + $ip = $ip_cidr[0]; + if(in_array((int)strlen((string)$ip_cidr[1]),array(1,2))) { + $ip_cidr[1] = \Froxlor\Validate\Validate::cidr2NetmaskAddr($org_ip); + } + $newfieldvalue[] = $ip . '/' . $ip_cidr[1]; + } else { + $newfieldvalue[] = $org_ip; + } + } + $newfieldvalue = implode(',', $newfieldvalue); + } $returnvalue = self::storeSettingField($fieldname, $fielddata, $newfieldvalue); if ($returnvalue !== false && is_array($fielddata) && isset($fielddata['settinggroup']) && $fielddata['settinggroup'] == 'system' && isset($fielddata['varname']) && $fielddata['varname'] == 'mysql_access_host') { diff --git a/lib/Froxlor/Validate/Validate.php b/lib/Froxlor/Validate/Validate.php index 150bf44a..b154439c 100644 --- a/lib/Froxlor/Validate/Validate.php +++ b/lib/Froxlor/Validate/Validate.php @@ -16,10 +16,10 @@ class Validate * @param * string language id for the error * @return string the clean string - * + * * If the default pattern is used and the string does not match, we try to replace the * 'bad' values and log the action. - * + * */ public static function validate($str, $fieldname, $pattern = '', $lng = '', $emptydefault = array(), $throw_exception = false) { @@ -64,6 +64,26 @@ class Validate exit(); } + /** + * Converts CIDR to a netmask address + * + * @thx to https://stackoverflow.com/a/5711080/3020926 + * @param string $cidr + * + * @return string + */ + public static function cidr2NetmaskAddr ($cidr) { + + $ta = substr ($cidr, strpos ($cidr, '/') + 1) * 1; + $netmask = str_split (str_pad (str_pad ('', $ta, '1'), 32, '0'), 8); + + foreach ($netmask as &$element) { + $element = bindec ($element); + } + + return join ('.', $netmask); + } + /** * Checks whether it is a valid ip * @@ -79,7 +99,7 @@ class Validate * whether to allow private network addresses * @param bool $allow_cidr * whether to allow CIDR values e.g. 10.10.10.10/16 - * + * * @return string|bool ip address on success, false on failure */ public static function validate_ip2($ip, $return_bool = false, $lng = 'invalidip', $allow_localhost = false, $allow_priv = false, $allow_cidr = false, $throw_exception = false) @@ -90,6 +110,9 @@ class Validate $ip_cidr = explode("/", $ip); if (count($ip_cidr) == 2) { $ip = $ip_cidr[0]; + if(in_array((int)strlen((string)$ip_cidr[1]),array(1,2))) { + $ip_cidr[1] = self::cidr2NetmaskAddr($org_ip); + } $cidr = "/" . $ip_cidr[1]; } else { $ip = $org_ip; @@ -129,7 +152,7 @@ class Validate * The domainname which should be checked. * @param bool $allow_underscore * optional if true, allowes the underscore character in a domain label (DKIM etc.) - * + * * @return string|boolean the domain-name if the domain is valid, false otherwise */ public static function validateDomain($domainname, $allow_underscore = false) @@ -184,7 +207,7 @@ class Validate * string The username to check * @return bool Correct or not * @author Michael Duergner - * + * */ public static function validateUsername($username, $unix_names = 1, $mysql_max = '') { diff --git a/lng/english.lng.php b/lng/english.lng.php index 672df707..964ec0b3 100644 --- a/lng/english.lng.php +++ b/lng/english.lng.php @@ -569,7 +569,7 @@ $lng['serversettings']['apacheconf_htpasswddir']['description'] = 'Where should $lng['error']['formtokencompromised'] = 'The request seems to be compromised. For security reasons you were logged out.'; $lng['serversettings']['mysql_access_host']['title'] = 'MySQL-Access-Hosts'; -$lng['serversettings']['mysql_access_host']['description'] = 'A comma separated list of hosts from which users should be allowed to connect to the MySQL-Server. To allow a subnet, long CIDR syntax like 100.127.0.0/255.255.0.0 is valid.'; +$lng['serversettings']['mysql_access_host']['description'] = 'A comma separated list of hosts from which users should be allowed to connect to the MySQL-Server. To allow a subnet the netmask or cidr syntax is valid.'; // ADDED IN 1.2.18-svn1 diff --git a/lng/german.lng.php b/lng/german.lng.php index bcb57709..791c2513 100644 --- a/lng/german.lng.php +++ b/lng/german.lng.php @@ -564,7 +564,7 @@ $lng['serversettings']['apacheconf_htpasswddir']['description'] = 'Wo sollen die $lng['error']['formtokencompromised'] = 'Das Formular scheint manipuliert worden zu sein. Aus Sicherheitsgründen wurden Sie ausgelogged.'; $lng['serversettings']['mysql_access_host']['title'] = 'MySQL-Access-Hosts'; -$lng['serversettings']['mysql_access_host']['description'] = 'Eine durch Komma getrennte Liste mit den Hostnamen aller Hostnames/IP-Adressen, von denen sich die Benutzer einloggen dürfen. Um ein Subnetz zu erlauben, die lange CIDR Schreibweise (Beispiel 100.127.0.0/255.255.0.0) ist erlaubt.'; +$lng['serversettings']['mysql_access_host']['description'] = 'Eine durch Komma getrennte Liste mit den Hostnamen aller Hostnames/IP-Adressen, von denen sich die Benutzer einloggen dürfen. Um ein Subnetz zu erlauben ist die Netzmaske oder CIDR Syntax erlaubt.'; // ADDED IN 1.2.18-svn1 From c97cdb1c0e69fa5369efdc10a7f06436dcb096cd Mon Sep 17 00:00:00 2001 From: Pascal Date: Mon, 28 Oct 2019 13:15:48 +0100 Subject: [PATCH 04/16] make it more readable --- lib/Froxlor/Validate/Validate.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Froxlor/Validate/Validate.php b/lib/Froxlor/Validate/Validate.php index fa8e21af..3fd0d410 100644 --- a/lib/Froxlor/Validate/Validate.php +++ b/lib/Froxlor/Validate/Validate.php @@ -108,7 +108,7 @@ class Validate $ip_cidr = explode("/", $ip); if (count($ip_cidr) == 2) { $ip = $ip_cidr[0]; - if(in_array((int)strlen((string)$ip_cidr[1]),array(1,2))) { + if (strlen($ip_cidr[1]) <= 2) { $ip_cidr[1] = self::cidr2NetmaskAddr($org_ip); } $cidr = "/" . $ip_cidr[1]; From 84d1be538e9d20386e3654b59fadef7941af78ff Mon Sep 17 00:00:00 2001 From: Pascal Date: Mon, 28 Oct 2019 13:25:34 +0100 Subject: [PATCH 05/16] block ipv6 addresses in cidr notation (mysql can't handle it) --- lib/Froxlor/Validate/Validate.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/Froxlor/Validate/Validate.php b/lib/Froxlor/Validate/Validate.php index 3fd0d410..bf9ef268 100644 --- a/lib/Froxlor/Validate/Validate.php +++ b/lib/Froxlor/Validate/Validate.php @@ -106,7 +106,11 @@ class Validate if ($allow_cidr) { $org_ip = $ip; $ip_cidr = explode("/", $ip); - if (count($ip_cidr) == 2) { + if (count($ip_cidr) === 2) { + //MySQL does not handle CIDR of IPv6 addresses, return error + if (false === filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) { + \Froxlor\UI\Response::standard_error($lng, $ip, $throw_exception); + } $ip = $ip_cidr[0]; if (strlen($ip_cidr[1]) <= 2) { $ip_cidr[1] = self::cidr2NetmaskAddr($org_ip); From 83e932b068b3622d4cb04e3e7acc84215a838773 Mon Sep 17 00:00:00 2001 From: Pascal Date: Mon, 28 Oct 2019 13:26:32 +0100 Subject: [PATCH 06/16] switch join with implode --- lib/Froxlor/Validate/Validate.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Froxlor/Validate/Validate.php b/lib/Froxlor/Validate/Validate.php index bf9ef268..7b7b799a 100644 --- a/lib/Froxlor/Validate/Validate.php +++ b/lib/Froxlor/Validate/Validate.php @@ -79,7 +79,7 @@ class Validate $element = bindec ($element); } - return join ('.', $netmask); + return implode ('.', $netmask); } /** From e8d67f97118617cc7540ca82259c6730cbca6d5a Mon Sep 17 00:00:00 2001 From: Pascal Date: Mon, 28 Oct 2019 14:07:31 +0100 Subject: [PATCH 07/16] check if ipv6 first --- lib/Froxlor/Validate/Validate.php | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/Froxlor/Validate/Validate.php b/lib/Froxlor/Validate/Validate.php index 7b7b799a..0e23ad1c 100644 --- a/lib/Froxlor/Validate/Validate.php +++ b/lib/Froxlor/Validate/Validate.php @@ -82,6 +82,20 @@ class Validate return implode ('.', $netmask); } + /** + * Checks if an $address (IP) is IPv6 + * + * @param $address + * + * @return bool + * @thx https://stackoverflow.com/a/13677565/3020926 + */ + public static function is_ipv6($address) { + $ipv4_mapped_ipv6 = strpos($address, "::ffff:"); + return (strpos($address, ":") !== FALSE) && + ($ipv4_mapped_ipv6 === FALSE || $ipv4_mapped_ipv6 != 0); + } + /** * Checks whether it is a valid ip * @@ -107,9 +121,11 @@ class Validate $org_ip = $ip; $ip_cidr = explode("/", $ip); if (count($ip_cidr) === 2) { - //MySQL does not handle CIDR of IPv6 addresses, return error - if (false === filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) { - \Froxlor\UI\Response::standard_error($lng, $ip, $throw_exception); + if (self::is_ipv6($ip)) { + //MySQL does not handle CIDR of IPv6 addresses, return error + if (false === filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) { + \Froxlor\UI\Response::standard_error($lng, $ip, $throw_exception); + } } $ip = $ip_cidr[0]; if (strlen($ip_cidr[1]) <= 2) { From 220b493a1b3f48ba1c838ec75ab1fddd8177710e Mon Sep 17 00:00:00 2001 From: Pascal Date: Mon, 28 Oct 2019 14:16:27 +0100 Subject: [PATCH 08/16] better readability --- lib/Froxlor/Settings/Store.php | 4 ++-- tests/Froxlor/ValidateTest.php | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/Froxlor/Settings/Store.php b/lib/Froxlor/Settings/Store.php index 3371b921..56bb5fed 100644 --- a/lib/Froxlor/Settings/Store.php +++ b/lib/Froxlor/Settings/Store.php @@ -262,9 +262,9 @@ class Store foreach ($ips as $ip) { $org_ip = $ip; $ip_cidr = explode("/", $ip); - if (count($ip_cidr) == 2) { + if (count($ip_cidr) === 2) { $ip = $ip_cidr[0]; - if(in_array((int)strlen((string)$ip_cidr[1]),array(1,2))) { + if (strlen($ip_cidr[1]) <= 2) { $ip_cidr[1] = \Froxlor\Validate\Validate::cidr2NetmaskAddr($org_ip); } $newfieldvalue[] = $ip . '/' . $ip_cidr[1]; diff --git a/tests/Froxlor/ValidateTest.php b/tests/Froxlor/ValidateTest.php index a47dba82..6600d5f1 100644 --- a/tests/Froxlor/ValidateTest.php +++ b/tests/Froxlor/ValidateTest.php @@ -86,6 +86,13 @@ class ValidateTest extends TestCase $this->assertEquals("12.34.56.78/24", $result); } + public function testValidateIpv6Disallowed() + { + $this->expectException("Exception"); + $this->expectExceptionCode(400); + $result = Validate::validate_ip2("2620:0:2d0:200::7/32", false, 'invalidip', false, false, true, true); + } + public function testValidateIpLocalhostAllowed() { $result = Validate::validate_ip2("127.0.0.1/32", false, 'invalidip', true, false, true, true); From faf3abe800bc8ab82141c5ddb05c3e280700854b Mon Sep 17 00:00:00 2001 From: Pascal Date: Mon, 28 Oct 2019 15:33:26 +0100 Subject: [PATCH 09/16] introduce new parameter to allow automatic convert cidr notation to netmask notation --- lib/Froxlor/Validate/Check.php | 3 +-- lib/Froxlor/Validate/Validate.php | 42 ++++++++++++++++--------------- tests/Froxlor/ValidateTest.php | 10 ++++++++ 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/lib/Froxlor/Validate/Check.php b/lib/Froxlor/Validate/Check.php index 14f30fe3..041381f1 100644 --- a/lib/Froxlor/Validate/Check.php +++ b/lib/Froxlor/Validate/Check.php @@ -77,8 +77,7 @@ class Check $mysql_access_host_array = array_map('trim', explode(',', $newfieldvalue)); foreach ($mysql_access_host_array as $host_entry) { - - if (Validate::validate_ip2($host_entry, true, 'invalidip', true, true, true) == false && Validate::validateDomain($host_entry) == false && Validate::validateLocalHostname($host_entry) == false && $host_entry != '%') { + if (Validate::validate_ip2($host_entry, true, 'invalidip', true, true, true, false, true) == false && Validate::validateDomain($host_entry) == false && Validate::validateLocalHostname($host_entry) == false && $host_entry != '%') { return array( self::FORMFIELDS_PLAUSIBILITY_CHECK_ERROR, 'invalidmysqlhost', diff --git a/lib/Froxlor/Validate/Validate.php b/lib/Froxlor/Validate/Validate.php index 0e23ad1c..edc2a860 100644 --- a/lib/Froxlor/Validate/Validate.php +++ b/lib/Froxlor/Validate/Validate.php @@ -96,25 +96,27 @@ class Validate ($ipv4_mapped_ipv6 === FALSE || $ipv4_mapped_ipv6 != 0); } - /** - * Checks whether it is a valid ip - * - * @param string $ip - * ip-address to check - * @param bool $return_bool - * whether to return bool or call \Froxlor\UI\Response::standard_error() - * @param string $lng - * index for error-message (if $return_bool is false) - * @param bool $allow_localhost - * whether to allow 127.0.0.1 - * @param bool $allow_priv - * whether to allow private network addresses - * @param bool $allow_cidr - * whether to allow CIDR values e.g. 10.10.10.10/16 - * - * @return string|bool ip address on success, false on failure - */ - public static function validate_ip2($ip, $return_bool = false, $lng = 'invalidip', $allow_localhost = false, $allow_priv = false, $allow_cidr = false, $throw_exception = false) + /** + * Checks whether it is a valid ip + * + * @param string $ip + * ip-address to check + * @param bool $return_bool + * whether to return bool or call \Froxlor\UI\Response::standard_error() + * @param string $lng + * index for error-message (if $return_bool is false) + * @param bool $allow_localhost + * whether to allow 127.0.0.1 + * @param bool $allow_priv + * whether to allow private network addresses + * @param bool $allow_cidr + * whether to allow CIDR values e.g. 10.10.10.10/16 + * @param bool $cidr_as_netmask + * whether to format CIDR nodation to netmask notation + * + * @return string|bool ip address on success, false on failure + */ + public static function validate_ip2($ip, $return_bool = false, $lng = 'invalidip', $allow_localhost = false, $allow_priv = false, $allow_cidr = false, $throw_exception = false, $cidr_as_netmask = false) { $cidr = ""; if ($allow_cidr) { @@ -128,7 +130,7 @@ class Validate } } $ip = $ip_cidr[0]; - if (strlen($ip_cidr[1]) <= 2) { + if ($cidr_as_netmask && strlen($ip_cidr[1]) <= 2) { $ip_cidr[1] = self::cidr2NetmaskAddr($org_ip); } $cidr = "/" . $ip_cidr[1]; diff --git a/tests/Froxlor/ValidateTest.php b/tests/Froxlor/ValidateTest.php index 6600d5f1..0222b991 100644 --- a/tests/Froxlor/ValidateTest.php +++ b/tests/Froxlor/ValidateTest.php @@ -99,6 +99,16 @@ class ValidateTest extends TestCase $this->assertEquals("127.0.0.1/32", $result); } + public function testValidateCidrNoationToNetmaskNotationIPv4() + { + $result = Validate::validate_ip2("1.1.1.1/4", false, 'invalidip', true, false, true, true, true); + $this->assertEquals("1.1.1.1/240.0.0.0", $result); + $result = Validate::validate_ip2("8.8.8.8/18", false, 'invalidip', true, false, true, true, true); + $this->assertEquals("8.8.8.8/255.255.192.0", $result); + $result = Validate::validate_ip2("8.8.8.8/1", false, 'invalidip', true, false, true, true, true); + $this->assertEquals("8.8.8.8/128.0.0.0", $result); + } + public function testValidateIpLocalhostAllowedWrongIp() { $this->expectException("Exception"); From 686c2ae534f699e2030b45bf499583e984eb02f2 Mon Sep 17 00:00:00 2001 From: Pascal Date: Mon, 28 Oct 2019 16:00:43 +0100 Subject: [PATCH 10/16] fix comparison --- lib/Froxlor/Validate/Validate.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Froxlor/Validate/Validate.php b/lib/Froxlor/Validate/Validate.php index edc2a860..b32643bc 100644 --- a/lib/Froxlor/Validate/Validate.php +++ b/lib/Froxlor/Validate/Validate.php @@ -125,7 +125,7 @@ class Validate if (count($ip_cidr) === 2) { if (self::is_ipv6($ip)) { //MySQL does not handle CIDR of IPv6 addresses, return error - if (false === filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) { + if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) === false) { \Froxlor\UI\Response::standard_error($lng, $ip, $throw_exception); } } From 2ed0cad27b3065b54451056fa6f7971bf76f5e80 Mon Sep 17 00:00:00 2001 From: Pascal Date: Mon, 28 Oct 2019 16:27:54 +0100 Subject: [PATCH 11/16] #564: cidr notation can only be 1 through 32 --- lib/Froxlor/Validate/Validate.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/Froxlor/Validate/Validate.php b/lib/Froxlor/Validate/Validate.php index b32643bc..012ce9ea 100644 --- a/lib/Froxlor/Validate/Validate.php +++ b/lib/Froxlor/Validate/Validate.php @@ -123,6 +123,9 @@ class Validate $org_ip = $ip; $ip_cidr = explode("/", $ip); if (count($ip_cidr) === 2) { + if(in_array((int)$ip_cidr[1], array_values(range(1, 32)), TRUE) === false) { + \Froxlor\UI\Response::standard_error($lng, $ip, $throw_exception); + } if (self::is_ipv6($ip)) { //MySQL does not handle CIDR of IPv6 addresses, return error if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) === false) { From 7774e7606d532cf8073d5fe4c5169838c11ebfe2 Mon Sep 17 00:00:00 2001 From: Pascal Date: Mon, 28 Oct 2019 16:29:53 +0100 Subject: [PATCH 12/16] dont check notated ips again --- lib/Froxlor/Validate/Validate.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Froxlor/Validate/Validate.php b/lib/Froxlor/Validate/Validate.php index 012ce9ea..d2ad8238 100644 --- a/lib/Froxlor/Validate/Validate.php +++ b/lib/Froxlor/Validate/Validate.php @@ -123,7 +123,7 @@ class Validate $org_ip = $ip; $ip_cidr = explode("/", $ip); if (count($ip_cidr) === 2) { - if(in_array((int)$ip_cidr[1], array_values(range(1, 32)), TRUE) === false) { + if(strlen($ip_cidr[1]) <= 2 && in_array((int)$ip_cidr[1], array_values(range(1, 32)), TRUE) === false) { \Froxlor\UI\Response::standard_error($lng, $ip, $throw_exception); } if (self::is_ipv6($ip)) { From 9c4d61984013c3f4c79bdd65542d8bc4a35c55a9 Mon Sep 17 00:00:00 2001 From: Pascal Date: Mon, 28 Oct 2019 16:32:52 +0100 Subject: [PATCH 13/16] remove inner if statement check ipv6 when cidr>netmask flag is set --- lib/Froxlor/Validate/Validate.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/Froxlor/Validate/Validate.php b/lib/Froxlor/Validate/Validate.php index d2ad8238..dcb53bab 100644 --- a/lib/Froxlor/Validate/Validate.php +++ b/lib/Froxlor/Validate/Validate.php @@ -126,11 +126,9 @@ class Validate if(strlen($ip_cidr[1]) <= 2 && in_array((int)$ip_cidr[1], array_values(range(1, 32)), TRUE) === false) { \Froxlor\UI\Response::standard_error($lng, $ip, $throw_exception); } - if (self::is_ipv6($ip)) { + if ($cidr_as_netmask && self::is_ipv6($ip)) { //MySQL does not handle CIDR of IPv6 addresses, return error - if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) === false) { - \Froxlor\UI\Response::standard_error($lng, $ip, $throw_exception); - } + \Froxlor\UI\Response::standard_error($lng, $ip, $throw_exception); } $ip = $ip_cidr[0]; if ($cidr_as_netmask && strlen($ip_cidr[1]) <= 2) { From d76f4108e5854a06beb3142b683f50834662bf48 Mon Sep 17 00:00:00 2001 From: Pascal Date: Mon, 28 Oct 2019 16:40:22 +0100 Subject: [PATCH 14/16] dont need $result if we're expecting an exception --- tests/Froxlor/ValidateTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Froxlor/ValidateTest.php b/tests/Froxlor/ValidateTest.php index 0222b991..dd711bfa 100644 --- a/tests/Froxlor/ValidateTest.php +++ b/tests/Froxlor/ValidateTest.php @@ -90,7 +90,7 @@ class ValidateTest extends TestCase { $this->expectException("Exception"); $this->expectExceptionCode(400); - $result = Validate::validate_ip2("2620:0:2d0:200::7/32", false, 'invalidip', false, false, true, true); + Validate::validate_ip2("2620:0:2d0:200::7/32", false, 'invalidip', false, false, true, true); } public function testValidateIpLocalhostAllowed() From 9689afc759e1e79a1c10f28afb8b0eab0e6c3a49 Mon Sep 17 00:00:00 2001 From: Pascal Date: Mon, 28 Oct 2019 16:58:34 +0100 Subject: [PATCH 15/16] change method signature of \Froxlor\Validate\Validate::validate_ip2 --- lib/Froxlor/Api/Commands/IpsAndPorts.php | 4 ++-- lib/Froxlor/Validate/Check.php | 2 +- lib/Froxlor/Validate/Validate.php | 5 ++++- tests/Froxlor/ValidateTest.php | 18 +++++++++--------- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/Froxlor/Api/Commands/IpsAndPorts.php b/lib/Froxlor/Api/Commands/IpsAndPorts.php index b0a9f05e..801d6f7c 100644 --- a/lib/Froxlor/Api/Commands/IpsAndPorts.php +++ b/lib/Froxlor/Api/Commands/IpsAndPorts.php @@ -135,7 +135,7 @@ class IpsAndPorts extends \Froxlor\Api\ApiCommand implements \Froxlor\Api\Resour { if ($this->isAdmin() && $this->getUserDetail('change_serversettings')) { - $ip = \Froxlor\Validate\Validate::validate_ip2($this->getParam('ip'), false, 'invalidip', false, false, false, true); + $ip = \Froxlor\Validate\Validate::validate_ip2($this->getParam('ip'), false, 'invalidip', false, false, false, false, true); $port = \Froxlor\Validate\Validate::validate($this->getParam('port', true, 80), 'port', '/^(([1-9])|([1-9][0-9])|([1-9][0-9][0-9])|([1-9][0-9][0-9][0-9])|([1-5][0-9][0-9][0-9][0-9])|(6[0-4][0-9][0-9][0-9])|(65[0-4][0-9][0-9])|(655[0-2][0-9])|(6553[0-5]))$/Di', array( 'stringisempty', 'myport' @@ -332,7 +332,7 @@ class IpsAndPorts extends \Froxlor\Api\ApiCommand implements \Froxlor\Api\Resour 'id' => $id )); - $ip = \Froxlor\Validate\Validate::validate_ip2($this->getParam('ip', true, $result['ip']), false, 'invalidip', false, false, false, true); + $ip = \Froxlor\Validate\Validate::validate_ip2($this->getParam('ip', true, $result['ip']), false, 'invalidip', false, false, false, false, true); $port = \Froxlor\Validate\Validate::validate($this->getParam('port', true, $result['port']), 'port', '/^(([1-9])|([1-9][0-9])|([1-9][0-9][0-9])|([1-9][0-9][0-9][0-9])|([1-5][0-9][0-9][0-9][0-9])|(6[0-4][0-9][0-9][0-9])|(65[0-4][0-9][0-9])|(655[0-2][0-9])|(6553[0-5]))$/Di', array( 'stringisempty', 'myport' diff --git a/lib/Froxlor/Validate/Check.php b/lib/Froxlor/Validate/Check.php index 041381f1..430e2e86 100644 --- a/lib/Froxlor/Validate/Check.php +++ b/lib/Froxlor/Validate/Check.php @@ -77,7 +77,7 @@ class Check $mysql_access_host_array = array_map('trim', explode(',', $newfieldvalue)); foreach ($mysql_access_host_array as $host_entry) { - if (Validate::validate_ip2($host_entry, true, 'invalidip', true, true, true, false, true) == false && Validate::validateDomain($host_entry) == false && Validate::validateLocalHostname($host_entry) == false && $host_entry != '%') { + if (Validate::validate_ip2($host_entry, true, 'invalidip', true, true, true, true, false) == false && Validate::validateDomain($host_entry) == false && Validate::validateLocalHostname($host_entry) == false && $host_entry != '%') { return array( self::FORMFIELDS_PLAUSIBILITY_CHECK_ERROR, 'invalidmysqlhost', diff --git a/lib/Froxlor/Validate/Validate.php b/lib/Froxlor/Validate/Validate.php index dcb53bab..5458755a 100644 --- a/lib/Froxlor/Validate/Validate.php +++ b/lib/Froxlor/Validate/Validate.php @@ -114,9 +114,12 @@ class Validate * @param bool $cidr_as_netmask * whether to format CIDR nodation to netmask notation * + * @param bool $throw_exception + * whether to throw an exception on failure + * * @return string|bool ip address on success, false on failure */ - public static function validate_ip2($ip, $return_bool = false, $lng = 'invalidip', $allow_localhost = false, $allow_priv = false, $allow_cidr = false, $throw_exception = false, $cidr_as_netmask = false) + public static function validate_ip2($ip, $return_bool = false, $lng = 'invalidip', $allow_localhost = false, $allow_priv = false, $allow_cidr = false, $cidr_as_netmask = false, $throw_exception = false) { $cidr = ""; if ($allow_cidr) { diff --git a/tests/Froxlor/ValidateTest.php b/tests/Froxlor/ValidateTest.php index dd711bfa..b825c057 100644 --- a/tests/Froxlor/ValidateTest.php +++ b/tests/Froxlor/ValidateTest.php @@ -50,7 +50,7 @@ class ValidateTest extends TestCase public function testValidateIp() { - $result = Validate::validate_ip2("12.34.56.78", false, 'invalidip', false, false, false, true); + $result = Validate::validate_ip2("12.34.56.78", false, 'invalidip', false, false, false, false, true); $this->assertEquals("12.34.56.78", $result); } @@ -58,12 +58,12 @@ class ValidateTest extends TestCase { $this->expectException("Exception"); $this->expectExceptionCode(400); - Validate::validate_ip2("10.0.0.1", false, 'invalidip', false, false, false, true); + Validate::validate_ip2("10.0.0.1", false, 'invalidip', false, false, false, false, true); } public function testValidateIpPrivNotAllowedBool() { - $result = Validate::validate_ip2("10.0.0.1", true, 'invalidip', false, false, false, true); + $result = Validate::validate_ip2("10.0.0.1", true, 'invalidip', false, false, false, false, true); $this->assertFalse($result); } @@ -71,18 +71,18 @@ class ValidateTest extends TestCase { $this->expectException("Exception"); $this->expectExceptionCode(400); - Validate::validate_ip2("12.34.56.78/24", false, 'invalidip', false, false, false, true); + Validate::validate_ip2("12.34.56.78/24", false, 'invalidip', false, false, false, false, true); } public function testValidateIpCidrNotAllowedBool() { - $result = Validate::validate_ip2("12.34.56.78/24", true, 'invalidip', false, false, false, true); + $result = Validate::validate_ip2("12.34.56.78/24", true, 'invalidip', false, false, false, false, true); $this->assertFalse($result); } public function testValidateIpCidr() { - $result = Validate::validate_ip2("12.34.56.78/24", false, 'invalidip', false, false, true, true); + $result = Validate::validate_ip2("12.34.56.78/24", false, 'invalidip', false, false, true, false, true); $this->assertEquals("12.34.56.78/24", $result); } @@ -90,12 +90,12 @@ class ValidateTest extends TestCase { $this->expectException("Exception"); $this->expectExceptionCode(400); - Validate::validate_ip2("2620:0:2d0:200::7/32", false, 'invalidip', false, false, true, true); + Validate::validate_ip2("2620:0:2d0:200::7/32", false, 'invalidip', false, false, true, true, true); } public function testValidateIpLocalhostAllowed() { - $result = Validate::validate_ip2("127.0.0.1/32", false, 'invalidip', true, false, true, true); + $result = Validate::validate_ip2("127.0.0.1/32", false, 'invalidip', true, false, true, false, true); $this->assertEquals("127.0.0.1/32", $result); } @@ -113,7 +113,7 @@ class ValidateTest extends TestCase { $this->expectException("Exception"); $this->expectExceptionCode(400); - Validate::validate_ip2("127.0.0.2", false, 'invalidip', true, false, false, true); + Validate::validate_ip2("127.0.0.2", false, 'invalidip', true, false, false, false, true); } public function testValidateUrl() From 5c11eecbd7959c6de1c5dab1615a14a0392f9a21 Mon Sep 17 00:00:00 2001 From: Pascal Date: Mon, 28 Oct 2019 17:27:39 +0100 Subject: [PATCH 16/16] remove code for checking ipv6 mapped ipv4 notation --- lib/Froxlor/Validate/Validate.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/Froxlor/Validate/Validate.php b/lib/Froxlor/Validate/Validate.php index 5458755a..2eb20ffa 100644 --- a/lib/Froxlor/Validate/Validate.php +++ b/lib/Froxlor/Validate/Validate.php @@ -88,12 +88,9 @@ class Validate * @param $address * * @return bool - * @thx https://stackoverflow.com/a/13677565/3020926 */ public static function is_ipv6($address) { - $ipv4_mapped_ipv6 = strpos($address, "::ffff:"); - return (strpos($address, ":") !== FALSE) && - ($ipv4_mapped_ipv6 === FALSE || $ipv4_mapped_ipv6 != 0); + return filter_var($address, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6); } /** @@ -129,7 +126,7 @@ class Validate if(strlen($ip_cidr[1]) <= 2 && in_array((int)$ip_cidr[1], array_values(range(1, 32)), TRUE) === false) { \Froxlor\UI\Response::standard_error($lng, $ip, $throw_exception); } - if ($cidr_as_netmask && self::is_ipv6($ip)) { + if ($cidr_as_netmask && self::is_ipv6($ip_cidr[0])) { //MySQL does not handle CIDR of IPv6 addresses, return error \Froxlor\UI\Response::standard_error($lng, $ip, $throw_exception); }