From f6d1700dd44f327eb56de733365f665e42c78718 Mon Sep 17 00:00:00 2001 From: datorik Date: Thu, 6 Aug 2026 12:41:37 +0300 Subject: [PATCH 1/4] Udp.Code.Firewall sorting --- lib/Cleantalk/Common/Firewall.php | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/lib/Cleantalk/Common/Firewall.php b/lib/Cleantalk/Common/Firewall.php index a87307c09..f08a3d7f5 100644 --- a/lib/Cleantalk/Common/Firewall.php +++ b/lib/Cleantalk/Common/Firewall.php @@ -33,6 +33,14 @@ class Firewall public $debug; public $debug_data = ''; + /** + * Statuses from the lowest priority to the highest. A result of a personal list is raised above + * the whole array by self::prioritize(). + * + * Note the position of PASS_SFW__BY_WHITELIST: it is the global (non-personal) white list of the + * cloud - good bots and the other common exclusions. It has to stay UNDER the DENY_* statuses, + * so a black listed User-Agent outweighs a good bot IP. + */ private $statuses_priority = array( // Lowest 'PASS_SFW', @@ -41,12 +49,12 @@ class Firewall 'PASS_ANTIFLOOD', 'PASS_ANTICRAWLER_UA', 'PASS_ANTICRAWLER', + 'PASS_SFW__BY_WHITELIST', 'DENY_ANTIFLOOD_UA', 'DENY_ANTIFLOOD', 'DENY_ANTICRAWLER_UA', 'DENY_ANTICRAWLER', 'DENY_SFW', - 'PASS_SFW__BY_WHITELIST', // Highest ); @@ -109,7 +117,11 @@ public function run() $results = array(); - // Checking + // Checking. + // Every module has to be run before any decision is made. An early exit here would hide the + // results of the modules below - e.g. a UA black list hit of the AntiCrawler would never be + // taken into account if the SFW had found the IP in the white list or in a trusted network. + // The whole picture is collected first, the decision is made by self::prioritize(). foreach ($this->fw_modules as $module) { if (isset($module->isExcluded) && $module->isExcluded) { continue; @@ -119,13 +131,10 @@ public function run() if ( ! empty($module_results)) { $results[$module->module_name] = $module_results; } - - if ($this->isWhitelisted($results)) { - // Break protection logic if it whitelisted or trusted network. - break; - } } + $this->isWhitelisted($results); + // Write Logs foreach ($this->fw_modules as $module) { if (array_key_exists($module->module_name, $results)) { From d052672c09cb9cc9ac7adab4f29b825bb73d1c16 Mon Sep 17 00:00:00 2001 From: datorik Date: Thu, 6 Aug 2026 15:38:12 +0300 Subject: [PATCH 2/4] Udp.Code.Firewall sorting --- lib/Cleantalk/Common/Firewall.php | 41 ++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/lib/Cleantalk/Common/Firewall.php b/lib/Cleantalk/Common/Firewall.php index f08a3d7f5..9d7c9b817 100644 --- a/lib/Cleantalk/Common/Firewall.php +++ b/lib/Cleantalk/Common/Firewall.php @@ -34,8 +34,14 @@ class Firewall public $debug_data = ''; /** - * Statuses from the lowest priority to the highest. A result of a personal list is raised above - * the whole array by self::prioritize(). + * Statuses from the lowest priority to the highest. The position in this array IS the priority, + * self::calculatePriority() does nothing but look the status up here. + * + * A result of the personal lists of the site owner carries the is_personal flag, which is not a + * part of the status itself - the very same DENY_SFW comes both from the personal and from the + * common list. Such results are listed here with the PERSONAL__ prefix, so that both variants + * can take their own place in the order. A personal status absent from the list falls back to + * the position of its common variant. * * Note the position of PASS_SFW__BY_WHITELIST: it is the global (non-personal) white list of the * cloud - good bots and the other common exclusions. It has to stay UNDER the DENY_* statuses, @@ -55,6 +61,8 @@ class Firewall 'DENY_ANTICRAWLER_UA', 'DENY_ANTICRAWLER', 'DENY_SFW', + 'PERSONAL__DENY_SFW', + 'PERSONAL__PASS_SFW__BY_WHITELIST', // Highest ); @@ -203,8 +211,7 @@ private function prioritize($results) foreach ($this->fw_modules as $module) { if (array_key_exists($module->module_name, $results)) { foreach ($results[$module->module_name] as $fw_result) { - $priority = array_search($fw_result['status'], $this->statuses_priority) + - (isset($fw_result['is_personal']) && $fw_result['is_personal'] ? count($this->statuses_priority) : 0); + $priority = $this->calculatePriority($fw_result); if ($priority >= $current_fw_result_priority) { $current_fw_result_priority = $priority; $result['status'] = TT::getArrayValueAsString($fw_result, 'status'); @@ -227,6 +234,32 @@ private function prioritize($results) return $result; } + /** + * Returns the position of a single firewall result in self::$statuses_priority. + * + * A result of a personal list is looked up by the PERSONAL__ prefixed status first, so it takes + * its own place in the order. If there is no such entry, the common variant is used. + * + * @param array $fw_result + * + * @return int + */ + private function calculatePriority($fw_result) + { + $status = TT::getArrayValueAsString($fw_result, 'status'); + + if (isset($fw_result['is_personal']) && $fw_result['is_personal']) { + $personal_priority = array_search('PERSONAL__' . $status, $this->statuses_priority); + if ($personal_priority !== false) { + return $personal_priority; + } + } + + $priority = array_search($status, $this->statuses_priority); + + return $priority === false ? 0 : $priority; + } + /** * Check the result if it whitelisted or trusted network * From 89036c67ca6205794b02b3ba8139aff4266e7fef Mon Sep 17 00:00:00 2001 From: datorik Date: Tue, 18 Aug 2026 15:15:17 +0300 Subject: [PATCH 3/4] Udp.Code.Security review. --- lib/Cleantalk/Common/Firewall.php | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/Cleantalk/Common/Firewall.php b/lib/Cleantalk/Common/Firewall.php index 9d7c9b817..9f017879e 100644 --- a/lib/Cleantalk/Common/Firewall.php +++ b/lib/Cleantalk/Common/Firewall.php @@ -141,7 +141,7 @@ public function run() } } - $this->isWhitelisted($results); + $this->setWhitelistedCookie($results); // Write Logs foreach ($this->fw_modules as $module) { @@ -261,13 +261,13 @@ private function calculatePriority($fw_result) } /** - * Check the result if it whitelisted or trusted network + * Set the white list cookie if any of the results is whitelisted or belongs to a trusted network. * * @param array $results * - * @return bool + * @return void */ - private function isWhitelisted($results) + private function setWhitelistedCookie($results) { global $apbct; @@ -284,12 +284,10 @@ private function isWhitelisted($results) Cookie::set('ct_sfw_ip_wl', $cookie_val, time() + 86400 * 30, '/', '', null, true, 'Lax'); } - return true; + return; } } } } - - return false; } } From 8f99338a889981216eaf4555d513f9e33f2b5bfb Mon Sep 17 00:00:00 2001 From: Aleksandr Banins Date: Tue, 18 Aug 2026 15:27:26 +0300 Subject: [PATCH 4/4] New comment Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- lib/Cleantalk/Common/Firewall.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cleantalk/Common/Firewall.php b/lib/Cleantalk/Common/Firewall.php index 9f017879e..55ad3aabc 100644 --- a/lib/Cleantalk/Common/Firewall.php +++ b/lib/Cleantalk/Common/Firewall.php @@ -235,7 +235,7 @@ private function prioritize($results) } /** - * Returns the position of a single firewall result in self::$statuses_priority. + * Returns the position of a single firewall result in $this->statuses_priority. * * A result of a personal list is looked up by the PERSONAL__ prefixed status first, so it takes * its own place in the order. If there is no such entry, the common variant is used.