From f6d1700dd44f327eb56de733365f665e42c78718 Mon Sep 17 00:00:00 2001 From: datorik Date: Thu, 6 Aug 2026 12:41:37 +0300 Subject: [PATCH 1/2] 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/2] 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 *