From 14dd1350df08cb1cc471a3abf75ebf241c48d192 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 23 Apr 2020 11:27:24 +0200 Subject: [PATCH 1/3] Strip whitespaces from the beginning and end of user input to prevent errors getting triggered due to users trying to add "empty" domains. Signed-off-by: DL6ER --- scripts/pi-hole/php/groups.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/pi-hole/php/groups.php b/scripts/pi-hole/php/groups.php index 2167295e..3a7d09d5 100644 --- a/scripts/pi-hole/php/groups.php +++ b/scripts/pi-hole/php/groups.php @@ -54,7 +54,7 @@ if ($_POST['action'] == 'get_groups') { } elseif ($_POST['action'] == 'add_group') { // Add new group try { - $names = explode(' ', $_POST['name']); + $names = explode(' ', trim($_POST['name'])); $stmt = $db->prepare('INSERT INTO "group" (name,description) VALUES (:name,:desc)'); if (!$stmt) { throw new Exception('While preparing statement: ' . $db->lastErrorMsg()); @@ -234,7 +234,7 @@ if ($_POST['action'] == 'get_groups') { } elseif ($_POST['action'] == 'add_client') { // Add new client try { - $ips = explode(' ', $_POST['ip']); + $ips = explode(' ', trim($_POST['ip'])); $stmt = $db->prepare('INSERT INTO client (ip,comment) VALUES (:ip,:comment)'); if (!$stmt) { throw new Exception('While preparing statement: ' . $db->lastErrorMsg()); @@ -410,7 +410,7 @@ if ($_POST['action'] == 'get_groups') { } elseif ($_POST['action'] == 'add_domain') { // Add new domain try { - $domains = explode(' ', $_POST['domain']); + $domains = explode(' ', trim($_POST['domain'])); $stmt = $db->prepare('INSERT INTO domainlist (domain,type,comment) VALUES (:domain,:type,:comment)'); if (!$stmt) { throw new Exception('While preparing statement: ' . $db->lastErrorMsg()); @@ -604,7 +604,7 @@ if ($_POST['action'] == 'get_groups') { } elseif ($_POST['action'] == 'add_adlist') { // Add new adlist try { - $addresses = explode(' ', $_POST['address']); + $addresses = explode(' ', trim($_POST['address'])); $stmt = $db->prepare('INSERT INTO adlist (address,comment) VALUES (:address,:comment)'); if (!$stmt) { From 7d1163ff9c232361f71e43bf4279c7856efbd891 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 23 Apr 2020 11:38:17 +0200 Subject: [PATCH 2/3] Try to be clearer with the "failed to add domain" message by showing the original input when idn_to_ascii() failed and by showing both, the original and the converted domain when the conversion result itself is invalid. Signed-off-by: DL6ER --- scripts/pi-hole/php/groups.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/scripts/pi-hole/php/groups.php b/scripts/pi-hole/php/groups.php index 3a7d09d5..0363f23b 100644 --- a/scripts/pi-hole/php/groups.php +++ b/scripts/pi-hole/php/groups.php @@ -427,8 +427,10 @@ if ($_POST['action'] == 'get_groups') { } foreach ($domains as $domain) { - // Convert domain name to IDNA ASCII form for international domains - $domain = idn_to_ascii($domain); + $input = $domain; + // Convert domain name to IDNA ASCII form for international domains if intl package is available + if (extension_loaded("intl")) + $domain = idn_to_ascii($domain); if(strlen($_POST['type']) === 2 && $_POST['type'][1] === 'W') { @@ -442,7 +444,13 @@ if ($_POST['action'] == 'get_groups') { $domain = strtolower($domain); if(filter_var($domain, FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME) === false) { - throw new Exception('Domain ' . htmlentities(utf8_encode($domain)) . 'is not a valid domain.'); + // This is the case when idn_to_ascii() modified the string + if($input !== $domain && strlen($domain) > 0) + throw new Exception('Domain ' . htmlentities($input) . ' (converted to "' . htmlentities(utf8_encode($domain)) . '") is not a valid domain.'); + elseif($input !== $domain) + throw new Exception('Domain ' . htmlentities($input) . ' is not a valid domain.'); + else + throw new Exception('Domain ' . htmlentities(utf8_encode($domain)) . ' is not a valid domain.'); } } From 77b421b1336a0570a3aeae3bfbea2a6498fdffdc Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 23 Apr 2020 13:47:50 +0200 Subject: [PATCH 3/3] Show how many domains have been added in case of an error. Signed-off-by: DL6ER --- scripts/pi-hole/php/groups.php | 52 ++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/scripts/pi-hole/php/groups.php b/scripts/pi-hole/php/groups.php index 0363f23b..6f1dfdd2 100644 --- a/scripts/pi-hole/php/groups.php +++ b/scripts/pi-hole/php/groups.php @@ -55,6 +55,8 @@ if ($_POST['action'] == 'get_groups') { // Add new group try { $names = explode(' ', trim($_POST['name'])); + $total = count($names); + $added = 0; $stmt = $db->prepare('INSERT INTO "group" (name,description) VALUES (:name,:desc)'); if (!$stmt) { throw new Exception('While preparing statement: ' . $db->lastErrorMsg()); @@ -66,12 +68,15 @@ if ($_POST['action'] == 'get_groups') { foreach ($names as $name) { if (!$stmt->bindValue(':name', $name, SQLITE3_TEXT)) { - throw new Exception('While binding name: ' . $db->lastErrorMsg()); + throw new Exception('While binding name: ' . $db->lastErrorMsg() . '
'. + 'Added ' . $added . " out of ". $total . " groups"); } if (!$stmt->execute()) { - throw new Exception('While executing: ' . $db->lastErrorMsg()); + throw new Exception('While executing: ' . $db->lastErrorMsg() . '
'. + 'Added ' . $added . " out of ". $total . " groups"); } + $added++; } $reload = true; @@ -235,6 +240,8 @@ if ($_POST['action'] == 'get_groups') { // Add new client try { $ips = explode(' ', trim($_POST['ip'])); + $total = count($ips); + $added = 0; $stmt = $db->prepare('INSERT INTO client (ip,comment) VALUES (:ip,:comment)'); if (!$stmt) { throw new Exception('While preparing statement: ' . $db->lastErrorMsg()); @@ -251,12 +258,15 @@ if ($_POST['action'] == 'get_groups') { $comment = null; } if (!$stmt->bindValue(':comment', $comment, SQLITE3_TEXT)) { - throw new Exception('While binding comment: ' . $db->lastErrorMsg()); + throw new Exception('While binding comment: ' . $db->lastErrorMsg() . '
'. + 'Added ' . $added . " out of ". $total . " clients"); } if (!$stmt->execute()) { - throw new Exception('While executing: ' . $db->lastErrorMsg()); + throw new Exception('While executing: ' . $db->lastErrorMsg() . '
'. + 'Added ' . $added . " out of ". $total . " clients"); } + $added++; } $reload = true; @@ -411,6 +421,8 @@ if ($_POST['action'] == 'get_groups') { // Add new domain try { $domains = explode(' ', trim($_POST['domain'])); + $total = count($domains); + $added = 0; $stmt = $db->prepare('INSERT INTO domainlist (domain,type,comment) VALUES (:domain,:type,:comment)'); if (!$stmt) { throw new Exception('While preparing statement: ' . $db->lastErrorMsg()); @@ -445,22 +457,26 @@ if ($_POST['action'] == 'get_groups') { if(filter_var($domain, FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME) === false) { // This is the case when idn_to_ascii() modified the string - if($input !== $domain && strlen($domain) > 0) - throw new Exception('Domain ' . htmlentities($input) . ' (converted to "' . htmlentities(utf8_encode($domain)) . '") is not a valid domain.'); - elseif($input !== $domain) - throw new Exception('Domain ' . htmlentities($input) . ' is not a valid domain.'); - else - throw new Exception('Domain ' . htmlentities(utf8_encode($domain)) . ' is not a valid domain.'); + if($input !== $domain && strlen($domain) > 0) + $errormsg = 'Domain ' . htmlentities($input) . ' (converted to "' . htmlentities(utf8_encode($domain)) . '") is not a valid domain.'; + elseif($input !== $domain) + $errormsg = 'Domain ' . htmlentities($input) . ' is not a valid domain.'; + else + $errormsg = 'Domain ' . htmlentities(utf8_encode($domain)) . ' is not a valid domain.'; + throw new Exception($errormsg . '
'. 'Added ' . $added . " out of ". $total . " domains"); } } if (!$stmt->bindValue(':domain', $domain, SQLITE3_TEXT)) { - throw new Exception('While binding domain: ' . $db->lastErrorMsg()); + throw new Exception('While binding domain: ' . $db->lastErrorMsg() . '
'. + 'Added ' . $added . " out of ". $total . " domains"); } if (!$stmt->execute()) { - throw new Exception('While executing: ' . $db->lastErrorMsg()); + throw new Exception('While executing: ' . $db->lastErrorMsg() . '
'. + 'Added ' . $added . " out of ". $total . " domains"); } + $added++; } $reload = true; @@ -613,6 +629,8 @@ if ($_POST['action'] == 'get_groups') { // Add new adlist try { $addresses = explode(' ', trim($_POST['address'])); + $total = count($addresses); + $added = 0; $stmt = $db->prepare('INSERT INTO adlist (address,comment) VALUES (:address,:comment)'); if (!$stmt) { @@ -625,16 +643,20 @@ if ($_POST['action'] == 'get_groups') { foreach ($addresses as $address) { if(preg_match("/[^a-zA-Z0-9:\/?&%=~._-]/", $address) !== 0) { - throw new Exception('Invalid adlist URL'); + throw new Exception('Invalid adlist URL ' . htmlentities($address) . '
'. + 'Added ' . $added . " out of ". $total . " adlists"); } if (!$stmt->bindValue(':address', $address, SQLITE3_TEXT)) { - throw new Exception('While binding address: ' . $db->lastErrorMsg()); + throw new Exception('While binding address: ' . $db->lastErrorMsg() . '
'. + 'Added ' . $added . " out of ". $total . " adlists"); } if (!$stmt->execute()) { - throw new Exception('While executing: ' . $db->lastErrorMsg()); + throw new Exception('While executing: ' . $db->lastErrorMsg() . '
'. + 'Added ' . $added . " out of ". $total . " adlists"); } + $added++; } $reload = true;