From 2d58194621a23d91bb012d33320df82b0c0bec7d Mon Sep 17 00:00:00 2001
From: AntonV1211 <antonV1211@yandex.ru>
Date: Wed, 23 Sep 2026 14:45:46 +0700
Subject: [PATCH 01/17] Fix. Code. Optimizing the parsing of approved hashes
 https://app.doboard.com/1/task/57727

---
 .../BinaryCheckModule/BinaryCheckModule.php   | 126 +++++------
 lib/CleantalkSP/SpbctWP/Scanner/Helper.php    |  39 ----
 .../SpbctWP/Scanner/RemoteHashesReader.php    | 201 ++++++++++++++++++
 .../SpbctWP/Scanner/ScannerQueue.php          |  87 ++++----
 lib/CleantalkSP/SpbctWP/Scanner/Surface.php   |  85 ++++++--
 5 files changed, 361 insertions(+), 177 deletions(-)
 create mode 100644 lib/CleantalkSP/SpbctWP/Scanner/RemoteHashesReader.php

diff --git a/lib/CleantalkSP/SpbctWP/Scanner/BinaryCheckModule/BinaryCheckModule.php b/lib/CleantalkSP/SpbctWP/Scanner/BinaryCheckModule/BinaryCheckModule.php
index fc0d6228f..8286e3183 100644
--- a/lib/CleantalkSP/SpbctWP/Scanner/BinaryCheckModule/BinaryCheckModule.php
+++ b/lib/CleantalkSP/SpbctWP/Scanner/BinaryCheckModule/BinaryCheckModule.php
@@ -19,13 +19,6 @@ class BinaryCheckModule
      */
     private $statuses = array();
 
-    /**
-     * @var string[]
-     */
-    private static $executable_mime_types = array(
-        'application/x-executable'
-    );
-
     /**
      * Runs the BinaryCheckModule to find malicious binary files in a single batch.
      *
@@ -186,47 +179,7 @@ private function incrementStatus($status)
 
     private function analyzeBinaryFile($binary_file_path)
     {
-        $first_bytes = $this->getBinaryBytes($binary_file_path);
-        if (false === $first_bytes) {
-            return false;
-        }
-
-        // Method 1: Check using ord() to get byte values
-        $byte1 = ord($first_bytes[0]);
-        $byte2 = ord($first_bytes[1]);
-        $byte3 = ord($first_bytes[2]);
-        $byte4 = ord($first_bytes[3]);
-
-        // Check for ELF (0x7F followed by "ELF" = 0x45 0x4C 0x46)
-        if ($byte1 === 0x7F && $byte2 === 0x45 && $byte3 === 0x4C && $byte4 === 0x46) {
-            return true; // ELF file detected
-        }
-
-        // Check for PE/MZ (Windows executable: 0x4D 0x5A = "MZ")
-        if ($byte1 === 0x4D && $byte2 === 0x5A) {
-            return true; // PE file detected
-        }
-
-        // Check for MACHO (macOS/iOS executables)
-        // 32-bit big-endian: 0xFE 0xED 0xFA 0xCE
-        // 32-bit little-endian: 0xCE 0xFA 0xED 0xFE
-        // 64-bit big-endian: 0xFE 0xED 0xFA 0xCF
-        // 64-bit little-endian: 0xCF 0xFA 0xED 0xFE
-        $macho_signatures = array(
-            array(0xFE, 0xED, 0xFA, 0xCE), // 32-bit big-endian
-            array(0xCE, 0xFA, 0xED, 0xFE), // 32-bit little-endian
-            array(0xFE, 0xED, 0xFA, 0xCF), // 64-bit big-endian
-            array(0xCF, 0xFA, 0xED, 0xFE), // 64-bit little-endian
-        );
-
-        foreach ($macho_signatures as $signature) {
-            if ($byte1 === $signature[0] && $byte2 === $signature[1] &&
-                $byte3 === $signature[2] && $byte4 === $signature[3]) {
-                return true; // MACHO file detected
-            }
-        }
-
-        return false;
+        return self::hasExecutableSignature($binary_file_path);
     }
 
     private function markAsCritical($binary_file_path)
@@ -246,13 +199,9 @@ private function markAsCritical($binary_file_path)
      * @param $filename
      * @return false|string String of bytes on success, false on failure.
      */
-    private function getBinaryBytes($filename)
+    private static function getBinaryBytes($filename)
     {
-        if (!file_exists($filename)) {
-            return false;
-        }
-
-        $handle = fopen($filename, 'rb');
+        $handle = @fopen($filename, 'rb');
         if (!$handle) {
             return false;
         }
@@ -268,37 +217,74 @@ private function getBinaryBytes($filename)
     }
 
     /**
-     * Read MIME type and make decision if MIME is in expected static array.
+     * Check the file signature for the known executable formats.
+     *
+     * Reading 4 bytes is used instead of finfo_file() intentionally: finfo reads a large part of
+     * every file and makes the file system passes of the scanner dozens of times slower.
+     *
      * @param string $filename
-     * @param string $extension
+     *
      * @return bool
      */
-    public static function probablyBinaryExecutableFile($filename, $extension)
+    public static function hasExecutableSignature($filename)
     {
-        if (!is_string($filename) || !file_exists($filename)) {
+        $first_bytes = self::getBinaryBytes($filename);
+        if (false === $first_bytes) {
             return false;
         }
 
-        $extension = strtolower((string) $extension);
+        $byte1 = ord($first_bytes[0]);
+        $byte2 = ord($first_bytes[1]);
+        $byte3 = ord($first_bytes[2]);
+        $byte4 = ord($first_bytes[3]);
 
-        // only empty or elf extension
-        if (!in_array($extension, ['', 'elf'], true)) {
-            return false;
+        // ELF (0x7F followed by "ELF" = 0x45 0x4C 0x46)
+        if ($byte1 === 0x7F && $byte2 === 0x45 && $byte3 === 0x4C && $byte4 === 0x46) {
+            return true;
         }
 
-        if (function_exists('finfo_open') && function_exists('finfo_file')) {
-            $finfo = finfo_open(FILEINFO_MIME_TYPE);
-            if (!$finfo) {
-                return false;
-            }
-            $mime = finfo_file($finfo, $filename);
-            finfo_close($finfo);
+        // PE/MZ (Windows executable: 0x4D 0x5A = "MZ")
+        if ($byte1 === 0x4D && $byte2 === 0x5A) {
+            return true;
+        }
+
+        // MACHO (macOS/iOS executables)
+        $macho_signatures = array(
+            array(0xFE, 0xED, 0xFA, 0xCE), // 32-bit big-endian
+            array(0xCE, 0xFA, 0xED, 0xFE), // 32-bit little-endian
+            array(0xFE, 0xED, 0xFA, 0xCF), // 64-bit big-endian
+            array(0xCF, 0xFA, 0xED, 0xFE), // 64-bit little-endian
+        );
 
-            if (in_array($mime, self::$executable_mime_types, true)) {
+        foreach ($macho_signatures as $signature) {
+            if ($byte1 === $signature[0] && $byte2 === $signature[1] &&
+                $byte3 === $signature[2] && $byte4 === $signature[3]) {
                 return true;
             }
         }
 
         return false;
     }
+
+    /**
+     * Make decision if the file is an executable binary one.
+     * @param string $filename
+     * @param string $extension
+     * @return bool
+     */
+    public static function probablyBinaryExecutableFile($filename, $extension)
+    {
+        $extension = strtolower((string) $extension);
+
+        // only empty or elf extension, checked before any filesystem call
+        if (!in_array($extension, ['', 'elf'], true)) {
+            return false;
+        }
+
+        if (!is_string($filename) || $filename === '') {
+            return false;
+        }
+
+        return self::hasExecutableSignature($filename);
+    }
 }
diff --git a/lib/CleantalkSP/SpbctWP/Scanner/Helper.php b/lib/CleantalkSP/SpbctWP/Scanner/Helper.php
index b45b8cbcb..a936d6e40 100644
--- a/lib/CleantalkSP/SpbctWP/Scanner/Helper.php
+++ b/lib/CleantalkSP/SpbctWP/Scanner/Helper.php
@@ -121,45 +121,6 @@ public static function getHashesForModules($cms, $type, $plugin, $version)
         return array('error' => 'REMOTE_FILE_NOT_FOUND_OR_VERSION_IS_NOT_SUPPORTED__PLUG');
     }
 
-    /**
-     * Getting real hashs of approved files
-     *
-     * @param string $cms CMS name
-     * @param string $type Type - approved/rejected
-     * @param string $version
-     *
-     * @return false|string[]|non-empty-array<array-key, string> Array with all files hashes or Error Array
-     */
-    public static function getHashesForFiles($cms, $type, $version)
-    {
-        $file_path = 'https://cleantalk-security.s3-us-west-2.amazonaws.com/extensions_checksums/' . $cms . '/' . $type . '/' . $version . '.csv.gz';
-        if ( HTTP::getResponseCode($file_path) !== 200 ) {
-            return array('error' => 'REMOTE_FILE_NOT_FOUND');
-        }
-
-        $gz_data = HTTP::getContentFromURL($file_path, false);
-        if ( !empty($gz_data['error']) ) {
-            return (array) $gz_data;
-        }
-
-        if ( !function_exists('gzdecode') ) {
-            return array('error' => 'Function gzdecode not exists. Please update your PHP to version 5.4');
-        }
-
-        $data = gzdecode($gz_data);
-        if ( $data === false ) {
-            return array('error' => 'COULDNT_UNPACK');
-        }
-
-        /** @psalm-suppress UndefinedMethod */
-        $lines = CSV::parseCSVLite($data);
-        if ( count($lines) < 1 ) {
-            return array('error' => 'Empty hashes file');
-        }
-
-        return $lines;
-    }
-
     /**
      * Getting real hashes of CMS language (translation) files for all available translations
      *
diff --git a/lib/CleantalkSP/SpbctWP/Scanner/RemoteHashesReader.php b/lib/CleantalkSP/SpbctWP/Scanner/RemoteHashesReader.php
new file mode 100644
index 000000000..62c580644
--- /dev/null
+++ b/lib/CleantalkSP/SpbctWP/Scanner/RemoteHashesReader.php
@@ -0,0 +1,201 @@
+<?php
+
+namespace CleantalkSP\SpbctWP\Scanner;
+
+use CleantalkSP\SpbctWP\Helpers\HTTP;
+
+/**
+ * Reads the remote approved/denied hashes lists (gzipped CSV) by chunks.
+ *
+ * The lists contain hundreds of thousands of lines (~16 MB uncompressed for the approved one),
+ * so they are never held in memory as a whole string or array: the gzip stream is inflated
+ * block by block and the hashes are given to the caller by small chunks.
+ *
+ * @package       CleantalkSP\SpbctWP\Scanner
+ */
+class RemoteHashesReader
+{
+    const URL_TEMPLATE = 'https://cleantalk-security.s3-us-west-2.amazonaws.com/extensions_checksums/%s/%s/%s.csv.gz';
+
+    /**
+     * Amount of the compressed data passed to the inflate context per iteration
+     */
+    const READ_BLOCK_SIZE = 262144;
+
+    /**
+     * @var string
+     */
+    private $url;
+
+    /**
+     * @var string Compressed list, dropped as soon as it is inflated
+     */
+    private $gz_data = '';
+
+    /**
+     * @var string|null
+     */
+    private $error;
+
+    /**
+     * @var int
+     */
+    private $processed_count = 0;
+
+    /**
+     * @param string $cms
+     * @param string $type approved|denied
+     * @param string $version
+     */
+    public function __construct($cms, $type, $version)
+    {
+        $this->url = sprintf(self::URL_TEMPLATE, $cms, $type, $version);
+    }
+
+    /**
+     * Downloads the compressed list. Has to be called before the chunks() iteration.
+     *
+     * @return bool False on any error, see getError()
+     */
+    public function fetch()
+    {
+        if ( ! function_exists('inflate_init') ) {
+            $this->error = 'Required zlib functions are not available. Please enable the zlib PHP extension.';
+            return false;
+        }
+
+        if ( HTTP::getResponseCode($this->url) !== 200 ) {
+            $this->error = 'REMOTE_FILE_NOT_FOUND';
+            return false;
+        }
+
+        $gz_data = HTTP::getContentFromURL($this->url, false);
+
+        if ( is_array($gz_data) ) {
+            $this->error = ! empty($gz_data['error']) ? (string)$gz_data['error'] : 'REMOTE_FILE_BAD_RESPONSE';
+            return false;
+        }
+
+        if ( ! is_string($gz_data) || $gz_data === '' ) {
+            $this->error = 'Empty hashes file';
+            return false;
+        }
+
+        $this->gz_data = $gz_data;
+
+        return true;
+    }
+
+    /**
+     * Inflates the downloaded list and yields validated hashes by chunks.
+     *
+     * @param int $chunk_size
+     *
+     * @return \Generator
+     * @psalm-return \Generator<int, list<string>, mixed, void>
+     */
+    public function chunks($chunk_size = 5000)
+    {
+        $this->processed_count = 0;
+
+        $inflate_context = inflate_init(ZLIB_ENCODING_GZIP);
+        if ( $inflate_context === false ) {
+            $this->error = 'COULDNT_UNPACK';
+            return;
+        }
+
+        $gz_length = strlen($this->gz_data);
+        $chunk     = array();
+        $tail      = '';
+
+        for ( $offset = 0; $offset < $gz_length; $offset += self::READ_BLOCK_SIZE ) {
+            $is_last_block = $offset + self::READ_BLOCK_SIZE >= $gz_length;
+
+            $inflated = @inflate_add(
+                $inflate_context,
+                substr($this->gz_data, $offset, self::READ_BLOCK_SIZE),
+                $is_last_block ? ZLIB_FINISH : ZLIB_NO_FLUSH
+            );
+
+            if ( $inflated === false ) {
+                $this->gz_data = '';
+                $this->error   = 'COULDNT_UNPACK';
+                return;
+            }
+
+            $tail    .= $inflated;
+            $inflated = null;
+
+            $lines = explode("\n", $tail);
+            // the last element is an incomplete line, it will be completed by the next block
+            $tail  = (string)array_pop($lines);
+
+            foreach ( $lines as $line ) {
+                $hash = self::extractHash($line);
+                if ( $hash === null ) {
+                    continue;
+                }
+
+                $chunk[] = $hash;
+
+                if ( count($chunk) >= $chunk_size ) {
+                    $this->processed_count += count($chunk);
+                    yield $chunk;
+                    $chunk = array();
+                }
+            }
+        }
+
+        $this->gz_data = '';
+
+        // the very last line may come without the trailing newline
+        $hash = self::extractHash($tail);
+        if ( $hash !== null ) {
+            $chunk[] = $hash;
+        }
+
+        if ( count($chunk) ) {
+            $this->processed_count += count($chunk);
+            yield $chunk;
+        }
+    }
+
+    /**
+     * Amount of the hashes passed to the caller during the last chunks() iteration
+     *
+     * @return int
+     */
+    public function getProcessedCount()
+    {
+        return $this->processed_count;
+    }
+
+    /**
+     * @return string|null
+     */
+    public function getError()
+    {
+        return $this->error;
+    }
+
+    /**
+     * Gets the hash from the "path,hash" CSV line
+     *
+     * @param string $line
+     *
+     * @return string|null Null if the line is empty or malformed
+     */
+    private static function extractHash($line)
+    {
+        $line = trim($line);
+        if ( $line === '' ) {
+            return null;
+        }
+
+        $delimiter_position = strrpos($line, ',');
+        $hash               = $delimiter_position === false ? $line : substr($line, $delimiter_position + 1);
+
+        // the hashes are used in the raw SQL below, so anything but an alphanumeric string is skipped
+        return preg_match('#^[a-zA-Z0-9]+$#', $hash) ? $hash : null;
+    }
+}
diff --git a/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php b/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php
index 2c0957dff..b646d4cce 100644
--- a/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php
+++ b/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php
@@ -1023,16 +1023,18 @@ public function file_system_analysis($offset = null, $amount = null, $path_to_sc
             $sql_hat                     = 'INSERT INTO ' . SPBC_TBL_SCAN_FILES
                                            . ' (`path`, `size`, `perms`, `mtime`, `fast_hash`, `full_hash`, `detected_at`, `checked_heuristic`, `source`) VALUES ';
 
+            $cure_log = new CureLog();
+            $restored_files = $cure_log->getRestoredFiles();
+            $restored_paths = array();
+            foreach ( $restored_files as $restored_file ) {
+                $restored_paths[$restored_file['real_path']] = true;
+            }
+            unset($restored_files);
+
             foreach ( $scanner->output_files as $_key => $file ) {
                 // skip restored files as is
-                $cure_log = new CureLog();
-                $restored = $cure_log->getRestoredFiles();
-                if (!empty($restored)) {
-                    foreach ($restored as $restored_file) {
-                        if ($restored_file['real_path'] === $file['path']) {
-                            continue 2;
-                        }
-                    }
+                if ( isset($restored_paths[$file['path']]) ) {
+                    continue;
                 }
                 $raw_path            = $file['path'];
                 $file['path']        = trim($this->db->prepare('%s', $file['path'])->getQuery(), '\'');
@@ -1319,29 +1321,16 @@ public function get_denied_hashes() // phpcs:ignore PSR1.Methods.CamelCapsMethod
 
         $warnings_before_iteration = ScanResultsTableActions::getFilesByCategory('critical');
 
-        $result = \CleantalkSP\SpbctWP\Scanner\Helper::getHashesForFiles('wordpress', 'denied', '1.0.0');
+        $hashes_reader = new RemoteHashesReader('wordpress', 'denied', '1.0.0');
+        $hashes_fetched = $hashes_reader->fetch();
 
         $scanning_stages_storage = new ScanningStagesStorage();
         $scanning_stages_storage->converter->loadCollection();
         $stage_data_obj = $scanning_stages_storage->getStage(GetDeniedHashes::class);
 
-        if ( empty($result['error']) ) {
-            $spbc->data['scanner']['checksums_count_ct'] = count($result);
-            $spbc->save('data');
-
-            /** @var list<string> $result_escaped */
-            $result_escaped = array_map(function ($v): string {
-                $value = is_scalar($v) ? (string) $v : '';
-                $escaped = esc_sql($value);
-                return is_string($escaped) ? $escaped : '';
-            }, $result);
-
-            foreach ( $result_escaped as $hash ) {
-                if ( ! preg_match('#^[a-zA-Z0-9]+$#', $hash) ) {
-                    return array('error' => 'BAD_PARAMS');
-                }
-            }
+        $denied_hashes_count = 0;
 
+        if ( $hashes_fetched ) {
             /**
              * Reset previously denied files; hashes still in the list will be re-marked below.
              * Avoids a single huge NOT IN (...) that can exceed max_allowed_packet.
@@ -1358,10 +1347,11 @@ public function get_denied_hashes() // phpcs:ignore PSR1.Methods.CamelCapsMethod
             );
 
             /**
-             * Update the db depend on current hashes list (chunked to avoid max_allowed_packet).
+             * Update the db depend on current hashes list (chunked to avoid max_allowed_packet
+             * and to keep the whole list out of the memory).
              */
             $updated_rows = 0;
-            foreach ( array_chunk($result_escaped, 5000) as $chunk ) {
+            foreach ( $hashes_reader->chunks(5000) as $chunk ) {
                 $where = implode('\',\'', $chunk);
                 $updated_rows += (int) $this->db->execute(
                     'UPDATE ' . SPBC_TBL_SCAN_FILES
@@ -1376,14 +1366,19 @@ public function get_denied_hashes() // phpcs:ignore PSR1.Methods.CamelCapsMethod
                 );
             }
 
-            $stage_data_obj->set('count_denied_hashes', count($result));
+            $denied_hashes_count = $hashes_reader->getProcessedCount();
+
+            $spbc->data['scanner']['checksums_count_ct'] = $denied_hashes_count;
+            $spbc->save('data');
+
+            $stage_data_obj->set('count_denied_hashes', $denied_hashes_count);
             $stage_data_obj->set('count_denied_hashes_in_db', $updated_rows);
             $scanning_stages_storage->saveToDb();
         }
 
         $out = array(
             'end'       => 1,
-            'processed' => empty($result['error']) ? count($result) : 0,
+            'processed' => $denied_hashes_count,
         );
 
         // Adding to log
@@ -1424,32 +1419,16 @@ public function get_approved_hashes() // phpcs:ignore PSR1.Methods.CamelCapsMeth
 
         $warnings_before_iteration = ScanResultsTableActions::getFilesByCategory('critical');
 
-        $result = \CleantalkSP\SpbctWP\Scanner\Helper::getHashesForFiles('wordpress', 'approved', '1.0.0');
+        $hashes_reader = new RemoteHashesReader('wordpress', 'approved', '1.0.0');
+        $hashes_fetched = $hashes_reader->fetch();
 
         $scanning_stages_storage = new ScanningStagesStorage();
         $scanning_stages_storage->converter->loadCollection();
         $stage_data_obj = $scanning_stages_storage->getStage(GetApprovedHashes::class);
 
-        $approved_hashes_count = empty($result['error']) ? count($result) : 0;
-
-        if ( empty($result['error']) ) {
-            /** @var list<string> $result_escaped */
-            $result_escaped = array_map(function ($v): string {
-                $value = is_scalar($v) ? (string) $v : '';
-                $escaped = esc_sql($value);
-                return is_string($escaped) ? $escaped : '';
-            }, $result);
-            unset($result);
-
-            $spbc->data['scanner']['checksums_count_ct'] = $approved_hashes_count;
-            $spbc->save('data');
-
-            foreach ( $result_escaped as $hash ) {
-                if ( ! preg_match('#^[a-zA-Z0-9]+$#', $hash) ) {
-                    return array('error' => 'BAD_PARAMS');
-                }
-            }
+        $approved_hashes_count = 0;
 
+        if ( $hashes_fetched ) {
             /**
              * Reset previously approved files; hashes still in the list will be re-marked below.
              * Avoids a single huge NOT IN (...) that can exceed max_allowed_packet.
@@ -1465,10 +1444,11 @@ public function get_approved_hashes() // phpcs:ignore PSR1.Methods.CamelCapsMeth
             );
 
             /**
-             * Update the db depend on current hashes list (chunked to avoid max_allowed_packet).
+             * Update the db depend on current hashes list (chunked to avoid max_allowed_packet
+             * and to keep the whole list out of the memory).
              */
             $updated_rows = 0;
-            foreach ( array_chunk($result_escaped, 5000) as $chunk ) {
+            foreach ( $hashes_reader->chunks(5000) as $chunk ) {
                 $where = implode('\',\'', $chunk);
                 $updated_rows += (int) $this->db->execute(
                     'UPDATE ' . SPBC_TBL_SCAN_FILES
@@ -1481,6 +1461,11 @@ public function get_approved_hashes() // phpcs:ignore PSR1.Methods.CamelCapsMeth
                 );
             }
 
+            $approved_hashes_count = $hashes_reader->getProcessedCount();
+
+            $spbc->data['scanner']['checksums_count_ct'] = $approved_hashes_count;
+            $spbc->save('data');
+
             $stage_data_obj->set('count_approved_hashes', $approved_hashes_count);
             $stage_data_obj->set('count_approved_hashes_in_db', $updated_rows);
             $scanning_stages_storage->saveToDb();
diff --git a/lib/CleantalkSP/SpbctWP/Scanner/Surface.php b/lib/CleantalkSP/SpbctWP/Scanner/Surface.php
index 4f18b6072..fe9fa08bb 100644
--- a/lib/CleantalkSP/SpbctWP/Scanner/Surface.php
+++ b/lib/CleantalkSP/SpbctWP/Scanner/Surface.php
@@ -542,13 +542,17 @@ public function countFilesInDir($main_path)
                     if ($fileInfo->isFile()) {
                         $filename = $fileInfo->getFileName();
                         $current_file_extension = pathinfo($filename, PATHINFO_EXTENSION);
-                        $real_path = $fileInfo->getRealPath();
+                        // getRealPath() resolves symlinks and is expensive, links are skipped above anyway
+                        $real_path = $fileInfo->getPathname();
 
-                        $probably_binary = BinaryCheckModule::probablyBinaryExecutableFile($real_path, $current_file_extension);
                         $skip_by_restrictions = $this->skipFileByRestrictions($current_file_extension, $real_path);
                         $skip_by_filename = ! empty($this->files_except) && in_array($fileInfo->getPathname(), $this->files_except, true);
 
-                        if (!$probably_binary && ($skip_by_restrictions || $skip_by_filename)) {
+                        // binary executables are counted even when the common filters reject them
+                        if (
+                            ($skip_by_restrictions || $skip_by_filename) &&
+                            ! BinaryCheckModule::probablyBinaryExecutableFile($real_path, $current_file_extension)
+                        ) {
                             continue;
                         }
 
@@ -627,7 +631,6 @@ public function getFileStructure($main_path, $iterator_result, $is_root_dir)
                     //
                     $current_file_extension = pathinfo($path, PATHINFO_EXTENSION);
 
-                    $probably_binary = BinaryCheckModule::probablyBinaryExecutableFile($path, $current_file_extension);
                     $skip_by_restrictions = $this->skipFileByRestrictions($current_file_extension, $path);
                     $skip_by_filename = false;
 
@@ -643,8 +646,11 @@ public function getFileStructure($main_path, $iterator_result, $is_root_dir)
                         $skip_by_filename = in_array(ltrim($relative_path, '/\\'), $files_except_array, true);
                     }
 
-                    //main skip logic check
-                    if (!$probably_binary && ($skip_by_restrictions || $skip_by_filename)) {
+                    // main skip logic check, binary executables are collected even when the common filters reject them
+                    if (
+                        ($skip_by_restrictions || $skip_by_filename) &&
+                        ! BinaryCheckModule::probablyBinaryExecutableFile($path, $current_file_extension)
+                    ) {
                         continue;
                     }
 
@@ -716,15 +722,12 @@ public function getFileStructure($main_path, $iterator_result, $is_root_dir)
     public function fileDetails($file_list, $path_offset)
     {
         global $wpdb, $spbc;
-
+        $stored_full_hashes = $this->getStoredFullHashes($file_list, $path_offset);
         foreach ( $file_list as $key => $val ) {
             // Cutting file's path, leave path from CMS ROOT to file
 
             // This order is important!!!
-            $this->output_files[$key]['path']  = substr(
-                $spbc->is_windows ? str_replace('/', '\\', $val['path']) : $val['path'],
-                $path_offset
-            );
+            $this->output_files[$key]['path']  = $this->getRelativePath($val['path'], $path_offset);
             $this->output_files[$key]['size']  = @filesize($val['path']);
             $this->output_files[$key]['perms'] = substr(decoct(@fileperms($val['path'])), 3);
             $mtime = @filemtime($val['path']);
@@ -757,12 +760,7 @@ public function fileDetails($file_list, $path_offset)
 
             $current_file_full_hash = md5_file($val['path']);
 
-            $sql = $wpdb->prepare(
-                'SELECT `full_hash` FROM ' . SPBC_TBL_SCAN_FILES . ' WHERE `fast_hash` = %s',
-                $fast_hash
-            );
-
-            $db_full_hash = $wpdb->get_var($sql);
+            $db_full_hash = isset($stored_full_hashes[$fast_hash]) ? $stored_full_hashes[$fast_hash] : null;
 
             if (
                 $db_full_hash
@@ -798,6 +796,59 @@ public function fileDetails($file_list, $path_offset)
         }
     }
 
+    /**
+     * Path from the CMS root to the file
+     *
+     * @param string $path
+     * @param int $path_offset
+     *
+     * @return string
+     */
+    private function getRelativePath($path, $path_offset)
+    {
+        global $spbc;
+
+        return substr($spbc->is_windows ? str_replace('/', '\\', $path) : $path, $path_offset);
+    }
+
+    /**
+     * Gets the stored full hashes for the whole files batch at once
+     *
+     * @param array $file_list
+     * @param int $path_offset
+     *
+     * @return array<string, string> fast_hash => full_hash
+     */
+    private function getStoredFullHashes($file_list, $path_offset)
+    {
+        global $wpdb;
+
+        $fast_hashes = array();
+        foreach ( $file_list as $val ) {
+            $fast_hashes[] = md5($this->getRelativePath($val['path'], $path_offset));
+        }
+
+        $stored_full_hashes = array();
+
+        foreach ( array_chunk($fast_hashes, 1000) as $chunk ) {
+            $placeholders = implode(',', array_fill(0, count($chunk), '%s'));
+            $rows         = $wpdb->get_results(
+                $wpdb->prepare(
+                    'SELECT `fast_hash`, `full_hash` FROM ' . SPBC_TBL_SCAN_FILES
+                    . ' WHERE `fast_hash` IN (' . $placeholders . ')',
+                    $chunk
+                ),
+                ARRAY_A
+            );
+
+            foreach ( (array)$rows as $row ) {
+                $stored_full_hashes[$row['fast_hash']] = $row['full_hash'];
+            }
+        }
+
+        return $stored_full_hashes;
+    }
+
     /**
      * Getting dir details
      *

From 99e828d0123fd50e9f0e06fee2f647b958fd856f Mon Sep 17 00:00:00 2001
From: AntonV1211 <antonV1211@yandex.ru>
Date: Wed, 23 Sep 2026 15:01:56 +0700
Subject: [PATCH 02/17] Fix psalm errors

---
 lib/CleantalkSP/Common/Helpers/CSV.php                 | 1 +
 lib/CleantalkSP/SpbctWP/Scanner/RemoteHashesReader.php | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/CleantalkSP/Common/Helpers/CSV.php b/lib/CleantalkSP/Common/Helpers/CSV.php
index 72b3279c6..5d91b9635 100644
--- a/lib/CleantalkSP/Common/Helpers/CSV.php
+++ b/lib/CleantalkSP/Common/Helpers/CSV.php
@@ -41,6 +41,7 @@ public static function sanitizeFromEmptyLines(&$buffer)
      * @param string $buffer
      *
      * @return string[]
+     * @psalm-suppress PossiblyUnusedMethod
      */
     public static function parseCSVLite($buffer)
     {
diff --git a/lib/CleantalkSP/SpbctWP/Scanner/RemoteHashesReader.php b/lib/CleantalkSP/SpbctWP/Scanner/RemoteHashesReader.php
index 62c580644..e521c3538 100644
--- a/lib/CleantalkSP/SpbctWP/Scanner/RemoteHashesReader.php
+++ b/lib/CleantalkSP/SpbctWP/Scanner/RemoteHashesReader.php
@@ -123,12 +123,11 @@ public function chunks($chunk_size = 5000)
                 return;
             }
 
-            $tail    .= $inflated;
-            $inflated = null;
+            $tail .= $inflated;
 
             $lines = explode("\n", $tail);
             // the last element is an incomplete line, it will be completed by the next block
-            $tail  = (string)array_pop($lines);
+            $tail  = array_pop($lines);
 
             foreach ( $lines as $line ) {
                 $hash = self::extractHash($line);
@@ -172,6 +171,7 @@ public function getProcessedCount()
 
     /**
      * @return string|null
+     * @psalm-suppress PossiblyUnusedMethod
      */
     public function getError()
     {

From 62c6eb42169cf3ed1752b5ea41752f4b9ed23f72 Mon Sep 17 00:00:00 2001
From: AntonV1211 <antonV1211@yandex.ru>
Date: Wed, 7 Oct 2026 16:01:14 +0700
Subject: [PATCH 03/17] Dev. Code. Copilot reviewe

---
 .../SpbctWP/Scanner/RemoteHashesReader.php    | 107 ++++++++++++++----
 .../SpbctWP/Scanner/ScannerQueue.php          |   8 +-
 2 files changed, 88 insertions(+), 27 deletions(-)

diff --git a/lib/CleantalkSP/SpbctWP/Scanner/RemoteHashesReader.php b/lib/CleantalkSP/SpbctWP/Scanner/RemoteHashesReader.php
index e521c3538..24f8447d1 100644
--- a/lib/CleantalkSP/SpbctWP/Scanner/RemoteHashesReader.php
+++ b/lib/CleantalkSP/SpbctWP/Scanner/RemoteHashesReader.php
@@ -86,6 +86,51 @@ public function fetch()
         return true;
     }
 
+    /**
+     * Inflates the whole stream once without exposing the data, so the caller can start writing
+     * to the database only when the list is known to be complete.
+     *
+     * @return bool False on any error, see getError()
+     */
+    public function validate()
+    {
+        $expected_length = $this->getExpectedRawLength();
+        $inflated_length = 0;
+
+        foreach ( $this->inflateStream() as $block ) {
+            $inflated_length += strlen($block);
+        }
+
+        if ( $this->error !== null ) {
+            return false;
+        }
+
+        // A truncated body inflates without an error, only the gzip trailer exposes it
+        if ( $expected_length > 0 && $inflated_length !== $expected_length ) {
+            $this->error = 'TRUNCATED_HASHES_FILE';
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * Uncompressed size stored in the gzip trailer.
+     *
+     * @return int 0 when it cannot be read or does not fit into an integer
+     */
+    private function getExpectedRawLength()
+    {
+        if ( strlen($this->gz_data) < 18 ) {
+            return 0;
+        }
+
+        $trailer = unpack('V', substr($this->gz_data, -4));
+        $length  = isset($trailer[1]) ? (int)$trailer[1] : 0;
+
+        return $length > 0 ? $length : 0;
+    }
+
     /**
      * Inflates the downloaded list and yields validated hashes by chunks.
      *
@@ -98,32 +143,11 @@ public function chunks($chunk_size = 5000)
     {
         $this->processed_count = 0;
 
-        $inflate_context = inflate_init(ZLIB_ENCODING_GZIP);
-        if ( $inflate_context === false ) {
-            $this->error = 'COULDNT_UNPACK';
-            return;
-        }
-
-        $gz_length = strlen($this->gz_data);
-        $chunk     = array();
-        $tail      = '';
-
-        for ( $offset = 0; $offset < $gz_length; $offset += self::READ_BLOCK_SIZE ) {
-            $is_last_block = $offset + self::READ_BLOCK_SIZE >= $gz_length;
-
-            $inflated = @inflate_add(
-                $inflate_context,
-                substr($this->gz_data, $offset, self::READ_BLOCK_SIZE),
-                $is_last_block ? ZLIB_FINISH : ZLIB_NO_FLUSH
-            );
-
-            if ( $inflated === false ) {
-                $this->gz_data = '';
-                $this->error   = 'COULDNT_UNPACK';
-                return;
-            }
+        $chunk = array();
+        $tail  = '';
 
-            $tail .= $inflated;
+        foreach ( $this->inflateStream() as $block ) {
+            $tail .= $block;
 
             $lines = explode("\n", $tail);
             // the last element is an incomplete line, it will be completed by the next block
@@ -159,6 +183,39 @@ public function chunks($chunk_size = 5000)
         }
     }
 
+    /**
+     * Inflates the gzip stream block by block, the decompressed list never exists as a whole.
+     *
+     * @return \Generator
+     * @psalm-return \Generator<int, string, mixed, void>
+     */
+    private function inflateStream()
+    {
+        $inflate_context = inflate_init(ZLIB_ENCODING_GZIP);
+        $gz_length       = strlen($this->gz_data);
+        $failed          = $inflate_context === false;
+
+        for ( $offset = 0; ! $failed && $offset < $gz_length; $offset += self::READ_BLOCK_SIZE ) {
+            $is_last_block = $offset + self::READ_BLOCK_SIZE >= $gz_length;
+
+            $inflated = @inflate_add(
+                $inflate_context,
+                substr($this->gz_data, $offset, self::READ_BLOCK_SIZE),
+                $is_last_block ? ZLIB_FINISH : ZLIB_NO_FLUSH
+            );
+
+            if ( $inflated === false ) {
+                $failed = true;
+            } else {
+                yield $inflated;
+            }
+        }
+
+        if ( $failed ) {
+            $this->error = 'COULDNT_UNPACK';
+        }
+    }
+
     /**
      * Amount of the hashes passed to the caller during the last chunks() iteration
      *
diff --git a/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php b/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php
index dda36a911..5a22c5ba5 100644
--- a/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php
+++ b/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php
@@ -2318,7 +2318,9 @@ public function get_denied_hashes() // phpcs:ignore PSR1.Methods.CamelCapsMethod
         $warnings_before_iteration = ScanResultsTableActions::getFilesByCategory('critical');
 
         $hashes_reader = new RemoteHashesReader('wordpress', 'denied', '1.0.0');
-        $hashes_fetched = $hashes_reader->fetch();
+        // the list is validated before the reset below: a truncated one would leave the already
+        // classified files unmarked
+        $hashes_fetched = $hashes_reader->fetch() && $hashes_reader->validate();
 
         $scanning_stages_storage = new ScanningStagesStorage();
         $scanning_stages_storage->converter->loadCollection();
@@ -2416,7 +2418,9 @@ public function get_approved_hashes() // phpcs:ignore PSR1.Methods.CamelCapsMeth
         $warnings_before_iteration = ScanResultsTableActions::getFilesByCategory('critical');
 
         $hashes_reader = new RemoteHashesReader('wordpress', 'approved', '1.0.0');
-        $hashes_fetched = $hashes_reader->fetch();
+        // the list is validated before the reset below: a truncated one would leave the already
+        // classified files unmarked
+        $hashes_fetched = $hashes_reader->fetch() && $hashes_reader->validate();
 
         $scanning_stages_storage = new ScanningStagesStorage();
         $scanning_stages_storage->converter->loadCollection();

From 35c99819b9a21b4cf41fe6d7b9934f7e8c2e72d5 Mon Sep 17 00:00:00 2001
From: AntonV1211 <antonV1211@yandex.ru>
Date: Wed, 7 Oct 2026 16:05:47 +0700
Subject: [PATCH 04/17] Dev. Code. Sync common lib

---
 lib/CleantalkSP/Common/Helpers/CSV.php | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/CleantalkSP/Common/Helpers/CSV.php b/lib/CleantalkSP/Common/Helpers/CSV.php
index 5d91b9635..72b3279c6 100644
--- a/lib/CleantalkSP/Common/Helpers/CSV.php
+++ b/lib/CleantalkSP/Common/Helpers/CSV.php
@@ -41,7 +41,6 @@ public static function sanitizeFromEmptyLines(&$buffer)
      * @param string $buffer
      *
      * @return string[]
-     * @psalm-suppress PossiblyUnusedMethod
      */
     public static function parseCSVLite($buffer)
     {

From c80d383b99ccfaf2dbd27cfc04cf57c178baa161 Mon Sep 17 00:00:00 2001
From: AntonV1211 <antonV1211@yandex.ru>
Date: Wed, 7 Oct 2026 16:38:39 +0700
Subject: [PATCH 05/17] Dev. Code. Sync common lib

---
 lib/CleantalkSP/Common/Helpers/CSV.php | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/CleantalkSP/Common/Helpers/CSV.php b/lib/CleantalkSP/Common/Helpers/CSV.php
index 72b3279c6..5d91b9635 100644
--- a/lib/CleantalkSP/Common/Helpers/CSV.php
+++ b/lib/CleantalkSP/Common/Helpers/CSV.php
@@ -41,6 +41,7 @@ public static function sanitizeFromEmptyLines(&$buffer)
      * @param string $buffer
      *
      * @return string[]
+     * @psalm-suppress PossiblyUnusedMethod
      */
     public static function parseCSVLite($buffer)
     {

From 83945f1eff268920a566227bd679f9b7b7cabfec Mon Sep 17 00:00:00 2001
From: svedge <svedge@users.noreply.github.com>
Date: Wed, 14 Oct 2026 16:15:11 +0500
Subject: [PATCH 06/17] Update version

---
 security-malware-firewall.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security-malware-firewall.php b/security-malware-firewall.php
index cc2bffedb..fcb5d0235 100644
--- a/security-malware-firewall.php
+++ b/security-malware-firewall.php
@@ -5,7 +5,7 @@
 Plugin URI: https://wordpress.org/plugins/security-malware-firewall/
 Description: Security & Malware scan by CleanTalk to protect your website from online threats and viruses. IP/Country FireWall, Web application FireWall. Detailed stats and logs to have full control.
 Author: CleanTalk Security
-Version: 2.190
+Version: 2.190.99-fix
 Author URI: https://cleantalk.org
 Text Domain: security-malware-firewall
 Domain Path: /i18n

From 76da8f3aa7165a94f6afa4d9cd332b9a1ab46a7a Mon Sep 17 00:00:00 2001
From: alexandergull <alex.g@cleantalk.org>
Date: Mon, 19 Oct 2026 12:12:49 +0500
Subject: [PATCH 07/17] Fix. Scanner. Now manual scan starts correctly even if
 automatic scan option is disabled.

---
 inc/spbc-scanner.php                          |   9 +-
 inc/spbc-wpcli.php                            |   2 +-
 .../SpbctWP/Scanner/ScannerQueue.php          |  23 +-
 .../Scanner/ScannerQueueLaunchBackground.php  | 205 ++++++++++++++++++
 4 files changed, 230 insertions(+), 9 deletions(-)
 create mode 100644 tests/CleantalkSP/SpbctWP/Scanner/ScannerQueueLaunchBackground.php

diff --git a/inc/spbc-scanner.php b/inc/spbc-scanner.php
index 7af8b3196..58a647019 100644
--- a/inc/spbc-scanner.php
+++ b/inc/spbc-scanner.php
@@ -19,17 +19,20 @@ function spbc_scanner__start()
     if ( ! spbc_check_ajax_referer('spbc_secret_nonce', 'security', false) ) {
         wp_send_json(array('error' => 'Nonce had been changed. Please, restart the scan.'));
     }
-    return spbc_scanner__launch();
+    // Not a cron run: the "Enable autoscanning" setting only governs the scheduled/automatic
+    // scanner. An explicit click on "Perform Scan" must start a scan regardless of it.
+    return spbc_scanner__launch(false);
 }
 
 /**
  * Cron|RemoteCall wrapper function for launchBackground
  *
+ * @param bool $is_cron_run Whether this is a scheduled/automatic run (see ScannerQueue::launchBackground()).
  * @return bool|string|string[]
  */
-function spbc_scanner__launch()
+function spbc_scanner__launch($is_cron_run = true)
 {
-    $result = \CleantalkSP\SpbctWP\Scanner\ScannerQueue::launchBackground();
+    $result = \CleantalkSP\SpbctWP\Scanner\ScannerQueue::launchBackground($is_cron_run);
 
     if (\CleantalkSP\SpbctWP\RemoteCalls::check()) {
         $result = empty($result['error'])
diff --git a/inc/spbc-wpcli.php b/inc/spbc-wpcli.php
index 80df8d842..9ca44208f 100644
--- a/inc/spbc-wpcli.php
+++ b/inc/spbc-wpcli.php
@@ -493,7 +493,7 @@ private function scannerRunBackgroundScan()
 
         Transaction::get('background_scanner')->clearTransactionTimer();
         Cron::removeTask('background_scan');
-        \CleantalkSP\SpbctWP\Scanner\ScannerQueue::launchBackground();
+        \CleantalkSP\SpbctWP\Scanner\ScannerQueue::launchBackground(false);
 
         WP_CLI::success(__('Background scan will start within 30 seconds. Make note, this could take up to 1 hour to complete the scan process in the background.', 'security_malware_firewall'));
     }
diff --git a/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php b/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php
index 752b3a31f..6c3e7a44d 100644
--- a/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php
+++ b/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php
@@ -167,22 +167,35 @@ public function __construct($stage = '', $offset = null, $amount = null, $root_d
      * Also set a cron task for each 30 seconds in case RC failing
      * Set a transaction 'background_scan' and pass it to RC and Cron task
      *
+     * @param bool $is_cron_run Whether this call comes from the scheduled/automatic scanner
+     *                          (cron, remote call, etc.). When true, the "Enable autoscanning"
+     *                          (scanner__auto_start) setting is honored. Pass false for explicit,
+     *                          user-initiated scans (manual "Perform Scan" button, WP-CLI, etc.)
+     *                          — that setting must not block an explicit request to scan now.
      * @return bool|string[]
      */
-    public static function launchBackground()
+    public static function launchBackground($is_cron_run = true)
     {
         global $spbc;
 
-        $transaction_id = Transaction::get('background_scanner', 3600 * 2)->perform();
-
+        // Resolve these checks BEFORE starting the transaction timer. Previously the timer
+        // was started unconditionally, so a blocked launch still left
+        // spbc_scanner__isScannerIsInProgress() reporting "in_progress" for up to 2 hours
+        // with nothing actually running (no cron task, no remote call) - the scan tab UI
+        // showed an endless progress bar that nothing could resolve except "Clear Scan Results".
         if (
             ! $spbc->moderate ||
-            ! $spbc->settings['scanner__auto_start'] ||
-            ! $transaction_id
+            ( $is_cron_run && ! $spbc->settings['scanner__auto_start'] )
         ) {
             return true;
         }
 
+        $transaction_id = Transaction::get('background_scanner', 3600 * 2)->perform();
+
+        if ( ! $transaction_id ) {
+            return true;
+        }
+
         self::clearRuntimeError();
         self::clearPaused();
         self::clearAborted();
diff --git a/tests/CleantalkSP/SpbctWP/Scanner/ScannerQueueLaunchBackground.php b/tests/CleantalkSP/SpbctWP/Scanner/ScannerQueueLaunchBackground.php
new file mode 100644
index 000000000..7fd174675
--- /dev/null
+++ b/tests/CleantalkSP/SpbctWP/Scanner/ScannerQueueLaunchBackground.php
@@ -0,0 +1,205 @@
+<?php
+
+use CleantalkSP\SpbctWP\Cron;
+use CleantalkSP\SpbctWP\Scanner\ScannerQueue;
+use CleantalkSP\SpbctWP\Transaction;
+
+/**
+ * Regression guard for the "stuck scan" bug.
+ *
+ * launchBackground() used to start the 'background_scanner' transaction timer BEFORE
+ * checking $spbc->moderate / the scanner__auto_start setting. When a launch was blocked
+ * by those checks the function returned early, so no cron task and no remote call were
+ * ever created - but the timer stayed set. spbc_scanner__isScannerIsInProgress() reads
+ * exactly that timer, so the scanner tab reported "in_progress" for the full 2h lease
+ * with nothing running behind it: an endless progress bar, a useless "Pause Scan" button
+ * and "Clear Scan Results" as the only way out.
+ *
+ * On top of that, "Perform Scan" shared the scheduled-scan code path, so disabling
+ * "Enable autoscanning" (scanner__auto_start) silently blocked manual scans too.
+ * $is_cron_run separates the two: the setting governs scheduled runs only.
+ *
+ * @covers \CleantalkSP\SpbctWP\Scanner\ScannerQueue::launchBackground
+ */
+class ScannerQueueLaunchBackgroundTest extends SpbcTestCase
+{
+    /**
+     * Mirrors Transaction::init() - 'spbc_transaction__' . $name . '_start_time'.
+     */
+    const TIMER_OPTION = 'spbc_transaction__background_scanner_start_time';
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->resetScannerState();
+
+        global $spbc;
+        // Both are preconditions for any launch; individual tests override them.
+        $spbc->moderate = true;
+        $spbc->settings['scanner__auto_start'] = 1;
+    }
+
+    protected function tearDown(): void
+    {
+        $this->resetScannerState();
+        parent::tearDown();
+    }
+
+    /**
+     * Drop every trace a previous launch could leave behind.
+     *
+     * The timer matters most: Transaction::perform() refuses to start a second
+     * generation while the 2h lease of the previous one is still running, so a
+     * leftover timer would make the next test see a falsy $transaction_id and
+     * silently bail out - masking whatever that test tries to prove.
+     *
+     * @return void
+     */
+    private function resetScannerState()
+    {
+        Transaction::get('background_scanner', 3600 * 2)->reset();
+        Cron::removeTask('background_scan');
+        ScannerQueue::clearCurrentParams();
+        delete_option(self::TIMER_OPTION);
+    }
+
+    /**
+     * @return int
+     */
+    private function getTransactionTimer()
+    {
+        return (int) get_option(self::TIMER_OPTION, 0);
+    }
+
+    /**
+     * A launch is "started" only when all three survive: the timer the UI polls,
+     * the fallback cron task and the queued first chunk.
+     *
+     * @return void
+     */
+    private function assertScanStarted($message)
+    {
+        $this->assertGreaterThan(
+            0,
+            $this->getTransactionTimer(),
+            $message . ': the transaction timer must be set'
+        );
+
+        $task = Cron::getTask('background_scan');
+        $this->assertIsArray($task, $message . ': the background_scan cron task must be created');
+        $this->assertSame(
+            'get_cms_hashes',
+            $task['params']['stage'],
+            $message . ': the scan must be queued from the first stage'
+        );
+
+        $params = ScannerQueue::getCurrentParams();
+        $this->assertIsArray($params, $message . ': the first chunk must be queued');
+        $this->assertSame('get_cms_hashes', $params['stage']);
+    }
+
+    /**
+     * A blocked launch must leave NO state at all - that is the whole bug.
+     *
+     * @return void
+     */
+    private function assertScanNotStarted($message)
+    {
+        $this->assertSame(
+            0,
+            $this->getTransactionTimer(),
+            $message . ': a blocked launch must not leave a stuck "in progress" timer'
+        );
+        $this->assertFalse(
+            Cron::getTask('background_scan'),
+            $message . ': a blocked launch must not create a cron task'
+        );
+        $this->assertNull(
+            ScannerQueue::getCurrentParams(),
+            $message . ': a blocked launch must not queue a chunk'
+        );
+    }
+
+    /**
+     * THE regression test: scheduled run + autoscanning off = nothing happens,
+     * and crucially nothing is left behind to fake an "in progress" scan.
+     */
+    public function testCronRunWithAutoStartDisabledLeavesNoStuckState()
+    {
+        global $spbc;
+        $spbc->settings['scanner__auto_start'] = 0;
+
+        ScannerQueue::launchBackground(true);
+
+        $this->assertScanNotStarted('Cron run with autoscanning disabled');
+    }
+
+    /**
+     * THE fix: "Perform Scan" must work while "Enable autoscanning" is off.
+     */
+    public function testManualRunIgnoresDisabledAutoStart()
+    {
+        global $spbc;
+        $spbc->settings['scanner__auto_start'] = 0;
+
+        ScannerQueue::launchBackground(false);
+
+        $this->assertScanStarted('Manual run with autoscanning disabled');
+    }
+
+    public function testCronRunWithAutoStartEnabledStartsScan()
+    {
+        ScannerQueue::launchBackground(true);
+
+        $this->assertScanStarted('Cron run with autoscanning enabled');
+    }
+
+    public function testManualRunWithAutoStartEnabledStartsScan()
+    {
+        ScannerQueue::launchBackground(false);
+
+        $this->assertScanStarted('Manual run with autoscanning enabled');
+    }
+
+    /**
+     * Guards the default value and the direction of the $is_cron_run flag: every
+     * existing call site (cron, remote call) omits the argument and must keep
+     * honoring the setting. An accidental default flip would silently turn the
+     * "Enable autoscanning" checkbox into a no-op.
+     */
+    public function testDefaultArgumentBehavesAsCronRun()
+    {
+        global $spbc;
+        $spbc->settings['scanner__auto_start'] = 0;
+
+        ScannerQueue::launchBackground();
+
+        $this->assertScanNotStarted('Default (omitted) argument');
+    }
+
+    /**
+     * The moderate check guards both paths and must also stay in front of the timer.
+     *
+     * @dataProvider isCronRunProvider
+     */
+    public function testNoModerateNeverStartsScan($is_cron_run)
+    {
+        global $spbc;
+        $spbc->moderate = false;
+
+        ScannerQueue::launchBackground($is_cron_run);
+
+        $this->assertScanNotStarted('Launch without moderate (is_cron_run: ' . var_export($is_cron_run, true) . ')');
+    }
+
+    /**
+     * @return array
+     */
+    public function isCronRunProvider()
+    {
+        return array(
+            'cron run' => array(true),
+            'manual run' => array(false),
+        );
+    }
+}

From 6868138f680419f18ef04cef0982013a5c0320eb Mon Sep 17 00:00:00 2001
From: alexandergull <alex.g@cleantalk.org>
Date: Mon, 19 Oct 2026 15:25:04 +0500
Subject: [PATCH 08/17] Fix. Scanner. Cure. Skip cure stage if option is
 currently disabled in settings.

---
 .../SpbctWP/Scanner/ScannerQueue.php          |  25 ++---
 .../SpbctWP/Scanner/Stages/CureStage.php      |  21 ++++
 .../Scanner/CureStageAutoCureGuard.php        | 100 ++++++++++++++++++
 3 files changed, 127 insertions(+), 19 deletions(-)
 create mode 100644 tests/CleantalkSP/SpbctWP/Scanner/CureStageAutoCureGuard.php

diff --git a/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php b/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php
index 6c3e7a44d..aab7082d7 100644
--- a/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php
+++ b/lib/CleantalkSP/SpbctWP/Scanner/ScannerQueue.php
@@ -128,8 +128,6 @@ class ScannerQueue
 
     public function __construct($stage = '', $offset = null, $amount = null, $root_dir = null)
     {
-        global $spbc;
-
          // Increasing Script execution time
         $current_time_limit = (int)ini_get('max_execution_time');
         if ($current_time_limit > 0 && $current_time_limit < 120) {
@@ -143,23 +141,12 @@ public function __construct($stage = '', $offset = null, $amount = null, $root_d
         $this->offset = isset($offset) ? (int)$offset : Request::getInt('offset');
         $this->root   = $root_dir ?: realpath(ABSPATH);
 
-        // Crunch for cure backups
-        if ( isset($spbc->settings['scanner__auto_cure']) && $spbc->settings['scanner__auto_cure'] == 0) {
-            unset(self::$stages['auto_cure_backup']);
-        }
-
-        // Skip send_heuristic_suspicious_files if setting is disabled
-        if ( isset($spbc->settings['scanner__schedule_send_heuristic_suspicious_files']) && $spbc->settings['scanner__schedule_send_heuristic_suspicious_files'] == 0) {
-            unset(self::$stages['schedule_send_heuristic_suspicious_files']);
-        }
-
-        if ( isset($spbc->settings['scanner__os_cron_analysis']) && $spbc->settings['scanner__os_cron_analysis'] == 0) {
-            unset(self::$stages['os_cron_analysis']);
-        }
-
-        if ( isset($spbc->settings['scanner__db_trigger_analysis']) && $spbc->settings['scanner__db_trigger_analysis'] == 0) {
-            unset(self::$stages['db_trigger_analysis']);
-        }
+        // NOTE: this used to unset() disabled stages from self::$stages here. Those calls were
+        // dead code - self::$stages is a list, so the stage names are VALUES and
+        // unset(self::$stages['auto_cure_backup']) removed nothing. Disabled stages are skipped
+        // by getNextStage(), which maps a stage to its 'scanner__<stage>' setting, plus a runtime
+        // guard inside the stage itself where the names do not match (auto_cure_backup) or where
+        // the stage is destructive (auto_cure, see CureStage::runStage()).
     }
 
     /**
diff --git a/lib/CleantalkSP/SpbctWP/Scanner/Stages/CureStage.php b/lib/CleantalkSP/SpbctWP/Scanner/Stages/CureStage.php
index 19234acc5..ef68c010d 100644
--- a/lib/CleantalkSP/SpbctWP/Scanner/Stages/CureStage.php
+++ b/lib/CleantalkSP/SpbctWP/Scanner/Stages/CureStage.php
@@ -45,6 +45,27 @@ public function runStage($offset, $amount)
     {
         global $spbc;
 
+        // Defense in depth: re-check the setting at EXECUTION time.
+        //
+        // ScannerQueue::getNextStage() evaluates scanner__auto_cure only when the PREVIOUS
+        // stage ends; the resulting chunk is then stored and replayed as-is by the cron
+        // controller (ScannerQueue::runControllerBackground() dispatches $self->{$self->stage}()
+        // straight from the queued params). So a scan that queued 'auto_cure' while curing was
+        // enabled would still cure after the setting is switched off - the window is the whole
+        // cron period, and unbounded while the scan sits paused.
+        //
+        // Curing rewrites user files irreversibly, so finish the stage without touching anything.
+        // 'end' must be returned explicitly: prepareAjaxOutput() derives it from "no files left
+        // to cure", which would never become true here and would spin the stage forever.
+        //
+        // Only the automatic stage is gated. Manual curing calls processCure() directly
+        // (see Cure::cureFile()) and is an explicit admin action, so it stays available.
+        if ( isset($spbc->settings['scanner__auto_cure']) && (int) $spbc->settings['scanner__auto_cure'] === 0 ) {
+            $this->stage_result = array('end' => 1);
+
+            return;
+        }
+
         //prepare facade log
         $scanning_stages_storage = new ScanningStagesStorage();
         $scanning_stages_storage->converter->loadCollection();
diff --git a/tests/CleantalkSP/SpbctWP/Scanner/CureStageAutoCureGuard.php b/tests/CleantalkSP/SpbctWP/Scanner/CureStageAutoCureGuard.php
new file mode 100644
index 000000000..46469f627
--- /dev/null
+++ b/tests/CleantalkSP/SpbctWP/Scanner/CureStageAutoCureGuard.php
@@ -0,0 +1,100 @@
+<?php
+
+use CleantalkSP\SpbctWP\Scanner\Stages\CureStage;
+
+/**
+ * Regression guard for automatic curing running with "Cure malware automatically" disabled.
+ *
+ * ScannerQueue::getNextStage() evaluates scanner__auto_cure only when the PREVIOUS stage
+ * ends. The resulting chunk is persisted and replayed as-is by the cron controller, which
+ * dispatches the queued stage directly. A scan that queued 'auto_cure' while curing was
+ * enabled therefore reached CureStage::runStage() even after the setting was switched off -
+ * and runStage() did not check the setting, so user files were rewritten anyway. The window
+ * is the whole cron period, and unbounded while the scan sits paused.
+ *
+ * The DB handle is a stub that throws on any use: that is the actual assertion that a
+ * disabled setting means "touch nothing at all", not merely "report an empty result".
+ *
+ * @covers \CleantalkSP\SpbctWP\Scanner\Stages\CureStage::runStage
+ */
+class CureStageAutoCureGuardTest extends SpbcTestCase
+{
+    /**
+     * A DB handle that must never be used while curing is disabled.
+     *
+     * @return object
+     */
+    private function getExplodingDb()
+    {
+        return new class {
+            public function fetchAll($query = '', $type = null)
+            {
+                throw new \LogicException('CureStage touched the database while auto cure was disabled');
+            }
+
+            public function fetch($query = '', $type = null)
+            {
+                throw new \LogicException('CureStage touched the database while auto cure was disabled');
+            }
+
+            public function execute($query = '')
+            {
+                throw new \LogicException('CureStage touched the database while auto cure was disabled');
+            }
+        };
+    }
+
+    /**
+     * THE regression test: the setting is turned off after the chunk was queued.
+     */
+    public function testRunStageDoesNothingWhenAutoCureDisabled()
+    {
+        global $spbc;
+        $spbc->settings['scanner__auto_cure'] = 0;
+
+        $cure_stage = new CureStage($this->getExplodingDb());
+
+        $cure_stage->runStage(0, 100);
+
+        $this->assertSame(
+            array('end' => 1),
+            $cure_stage->getStageResult(),
+            'A disabled auto cure must end the stage immediately'
+        );
+    }
+
+    /**
+     * 'end' must be truthy or ScannerQueue::runControllerBackground() never advances past
+     * auto_cure and the scan spins on this stage forever. Guards against "fixing" the early
+     * return by routing it through prepareAjaxOutput(), which derives 'end' from "no files
+     * left to cure" - a condition that can never become true while curing is disabled.
+     */
+    public function testDisabledAutoCureEndsTheStageInsteadOfLooping()
+    {
+        global $spbc;
+        $spbc->settings['scanner__auto_cure'] = 0;
+
+        $cure_stage = new CureStage($this->getExplodingDb());
+
+        $cure_stage->runStage(0, 100);
+
+        $result = $cure_stage->getStageResult();
+        $this->assertNotEmpty($result['end'], 'The stage must report itself as finished');
+    }
+
+    /**
+     * The guard must not over-block: with curing enabled the stage has to do its work.
+     * Reaching the exploding DB proves execution got past the early return.
+     */
+    public function testRunStageProceedsWhenAutoCureEnabled()
+    {
+        global $spbc;
+        $spbc->settings['scanner__auto_cure'] = 1;
+
+        $cure_stage = new CureStage($this->getExplodingDb());
+
+        // Offset 1 skips the offset===0 bookkeeping branch and goes straight to the file lookup.
+        $this->expectException(\LogicException::class);
+        $cure_stage->runStage(1, 10);
+    }
+}

From 61b6a0136a5d060e4230641b1e1c7c703aa846d0 Mon Sep 17 00:00:00 2001
From: alexandergull <alex.g@cleantalk.org>
Date: Tue, 20 Oct 2026 12:16:14 +0500
Subject: [PATCH 09/17] Fix. Cure. Fixed cure regex errors before writing file
 contents.

Reject unreadable source files and failed regular-expression replacements for both code cure actions, preserving the source and recording a cure error.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 lib/CleantalkSP/SpbctWP/Scanner/Cure.php | 26 +++++++++++++++---------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/lib/CleantalkSP/SpbctWP/Scanner/Cure.php b/lib/CleantalkSP/SpbctWP/Scanner/Cure.php
index dbfe6b2c2..590d7fbf5 100644
--- a/lib/CleantalkSP/SpbctWP/Scanner/Cure.php
+++ b/lib/CleantalkSP/SpbctWP/Scanner/Cure.php
@@ -225,17 +225,20 @@ public function actionPerformWithCode($object, $actions, $_file, $signature_id)
         try {
             foreach ( $actions as $action => $action_details ) {
                 if ( file_exists($object['file']) ) {
-                    $file_content = file_get_contents($object['file']);
+                    $file_content = @file_get_contents($object['file']);
                 } else {
                     return array('error' => 'File does not exists.');
                 }
+                if ( $file_content === false ) {
+                    return array('error' => 'Could not read file during cure.');
+                }
 
                 $is_regexp = \CleantalkSP\SpbctWP\Helpers\Helper::isRegexp($object['code']);
 
                 switch ( $action ) {
                     case 'delete':
                         $file_content = $is_regexp
-                            ? preg_replace(
+                            ? @preg_replace(
                                 $object['code'],
                                 "\n" . '/* ' . $spbc->data["wl_brandname"] . ': Malware was deleted: #' . $signature_id . '*/',
                                 $file_content,
@@ -247,23 +250,26 @@ public function actionPerformWithCode($object, $actions, $_file, $signature_id)
                                 $file_content
                             );
 
-                        $result = @file_put_contents($object['file'], $file_content);
-
                         break;
 
                     case 'replace_with':
                         $file_content = $is_regexp
-                            ? preg_replace($object['code'], $action_details, $file_content, 1)
+                            ? @preg_replace($object['code'], $action_details, $file_content, 1)
                             : str_replace($object['code'], $action_details, $file_content);
 
-                        $result = @file_put_contents($object['file'], $file_content);
-
                         break;
+                    default:
+                        continue 2;
                 }
-            }
 
-            if ( false === $result ) {
-                $result = array('error' => 'Permissions denied.');
+                if ( $file_content === null ) {
+                    return array('error' => 'Regular expression failed during cure. PCRE error: ' . preg_last_error());
+                }
+
+                $result = @file_put_contents($object['file'], $file_content);
+                if ( $result === false ) {
+                    return array('error' => 'Permissions denied.');
+                }
             }
         } catch (\Exception $e) {
             $result = array('error' => 'Something went wrong during cure: ' . $e->getMessage());

From 54f448314b31c5f6d5e4d8d9218f7b1d5a5665db Mon Sep 17 00:00:00 2001
From: AntonV1211 <antonV1211@yandex.ru>
Date: Thu, 22 Oct 2026 17:13:45 +0700
Subject: [PATCH 10/17] Version 2.191

---
 readme.txt                    | 2 +-
 security-malware-firewall.php | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/readme.txt b/readme.txt
index 752d3c0a4..d491f2b7c 100644
--- a/readme.txt
+++ b/readme.txt
@@ -4,7 +4,7 @@ Tags: security, firewall, malware, login, 2fa
 Requires at least: 5.0
 Tested up to: 7.1
 Requires PHP: 7.2
-Stable tag: 2.190
+Stable tag: 2.191
 License: GPLv2
 License URI: https://www.gnu.org/licenses/gpl-2.0.html
 
diff --git a/security-malware-firewall.php b/security-malware-firewall.php
index 2c0d17cca..8010142d2 100644
--- a/security-malware-firewall.php
+++ b/security-malware-firewall.php
@@ -5,7 +5,7 @@
 Plugin URI: https://wordpress.org/plugins/security-malware-firewall/
 Description: Security & Malware scan by CleanTalk to protect your website from online threats and viruses. IP/Country FireWall, Web application FireWall. Detailed stats and logs to have full control.
 Author: CleanTalk Security
-Version: 2.190.99-dev
+Version: 2.191
 Author URI: https://cleantalk.org
 Text Domain: security-malware-firewall
 Domain Path: /i18n

From 5d5e622ba6064fde7fc0b49ae526ae08c7718086 Mon Sep 17 00:00:00 2001
From: "github-actions[bot]" <github-actions[bot]@users.noreply.github.com>
Date: Thu, 22 Oct 2026 10:19:25 +0000
Subject: [PATCH 11/17] chore(changelog): rebuild readme for v2.191 [skip ci]

---
 changelog.txt | 3 +++
 readme.txt    | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/changelog.txt b/changelog.txt
index fb0a9627f..a03fc7f67 100644
--- a/changelog.txt
+++ b/changelog.txt
@@ -1,5 +1,8 @@
 == Changelog ==
 
+= 2.191 Oct 22 2026 =
+* Fix. Code. Optimizing the parsing of approved hashes https://app.doboard.com/1/task/57727
+
 = 2.190 Oct 09 2026 =
 * New. Security. Lost-password interface is under rate limit. (#755)
 * Upd. BFP. Lost password interface protected. (#743)
diff --git a/readme.txt b/readme.txt
index d491f2b7c..dbef0bcf5 100644
--- a/readme.txt
+++ b/readme.txt
@@ -376,6 +376,9 @@ Security FireWall can mitigate HTTP/HTTPS DDoS attacks. When an intruder makes G
 
 == Changelog ==
 
+= 2.191 Oct 22 2026 =
+* Fix. Code. Optimizing the parsing of approved hashes https://app.doboard.com/1/task/57727
+
 = 2.190 Oct 13 2026 =
 * New. Security. Lost-password interface is under rate limit. (#755)
 * Upd. BFP. Lost password interface protected. (#743)

From 126d86462848c6916e8911587cd69d5c9872ec91 Mon Sep 17 00:00:00 2001
From: alexandergull <alex.g@cleantalk.org>
Date: Thu, 22 Oct 2026 17:09:16 +0500
Subject: [PATCH 12/17] Fix. Settings. Links updated.

---
 lib/CleantalkSP/SpbctWP/Views/Settings.php | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/CleantalkSP/SpbctWP/Views/Settings.php b/lib/CleantalkSP/SpbctWP/Views/Settings.php
index 5c1f5c207..0c091cc13 100644
--- a/lib/CleantalkSP/SpbctWP/Views/Settings.php
+++ b/lib/CleantalkSP/SpbctWP/Views/Settings.php
@@ -210,10 +210,10 @@ private static function getBackendLogsLink()
 
         $link = '';
 
-        if ($spbc->key_is_ok && $spbc->user_token) {
+        if ($spbc->key_is_ok) {
             $link = LinkConstructor::buildSimpleLink(
                 'https://cleantalk.org/',
-                'my/backend_logs?user_token=' . $spbc->user_token
+                'my/backend_logs'
             );
         }
 
@@ -229,7 +229,7 @@ private static function getMyBillingLink()
         if ($spbc->key_is_ok) {
             $link = LinkConstructor::buildSimpleLink(
                 'https://cleantalk.org/',
-                'my/bill/security?package=1'
+                'help/extra-package-security'
             );
         }
 

From cc07db73126a466226e950628d1c0cb1707cc69a Mon Sep 17 00:00:00 2001
From: "github-actions[bot]" <github-actions[bot]@users.noreply.github.com>
Date: Fri, 23 Oct 2026 03:51:10 +0000
Subject: [PATCH 13/17] chore(changelog): rebuild readme for v2.191 [skip ci]

---
 changelog.txt | 4 ++++
 readme.txt    | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/changelog.txt b/changelog.txt
index a03fc7f67..405d071a2 100644
--- a/changelog.txt
+++ b/changelog.txt
@@ -1,6 +1,10 @@
 == Changelog ==
 
 = 2.191 Oct 22 2026 =
+* Fix. Settings. Links updated.
+* Fix. Cure. Fixed cure regex errors before writing file contents.
+* Fix. Scanner. Cure. Skip cure stage if option is currently disabled in settings.
+* Fix. Scanner. Now manual scan starts correctly even if automatic scan option is disabled.
 * Fix. Code. Optimizing the parsing of approved hashes https://app.doboard.com/1/task/57727
 
 = 2.190 Oct 09 2026 =
diff --git a/readme.txt b/readme.txt
index dbef0bcf5..6b773afe3 100644
--- a/readme.txt
+++ b/readme.txt
@@ -377,6 +377,10 @@ Security FireWall can mitigate HTTP/HTTPS DDoS attacks. When an intruder makes G
 == Changelog ==
 
 = 2.191 Oct 22 2026 =
+* Fix. Settings. Links updated.
+* Fix. Cure. Fixed cure regex errors before writing file contents.
+* Fix. Scanner. Cure. Skip cure stage if option is currently disabled in settings.
+* Fix. Scanner. Now manual scan starts correctly even if automatic scan option is disabled.
 * Fix. Code. Optimizing the parsing of approved hashes https://app.doboard.com/1/task/57727
 
 = 2.190 Oct 13 2026 =

From 9672d9723ae4f0bb300c84b120ddecbba9da6efc Mon Sep 17 00:00:00 2001
From: AntonV1211 <antonV1211@yandex.ru>
Date: Mon, 26 Oct 2026 14:31:45 +0700
Subject: [PATCH 14/17] Dev. Git. Added the Skills folder to .gitignore

---
 .gitignore | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index eff5a4137..8ba6b35f7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -26,4 +26,5 @@ psalm.xml
 /lib/CleantalkSP/Common/FSWatcher/Storage/data/
 /lib/CleantalkSP/Common/FSWatcher/logs/
 /lib/CleantalkSP/Common/Helpers/composer.json
-/tests/.phpunit.result.cache
\ No newline at end of file
+/tests/.phpunit.result.cache
+/skills/
\ No newline at end of file

From 3cabc748758b5d00821bc539a5a9983327543169 Mon Sep 17 00:00:00 2001
From: Alex <galyshev@cleantalk.org>
Date: Mon, 26 Oct 2026 16:32:46 +0500
Subject: [PATCH 15/17] Fix. 2FA. Flow updates. (#761)

* Fix. 2FA. Flow updates.

* Fix. 2FA. Keep user rate limiter UA charset-safe.

Truncating UA by bytes could split a multibyte character; wpdb rejects
queries with invalid text and the limiter fails open, so the per-user
2FA attempts cap could be bypassed. Keep only printable ASCII.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix. 2FA. Atomic step token attempts and claim.

Concurrent requests with the same step token could overwrite each
other's fail counter and both pass the TOTP replay check. Now each
attempt is reserved by compare-and-swap of the token record before the
code check, and a correct code logs in only if this request deletes
the reserved record. The TOTP time step is marked used after the claim.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix. 2FA. Revoke pending challenge on password reset.

A step token and e-mail code issued for the old password could still
complete the login after the password reset. Revoke both on
after_password_reset.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix. 2FA. Keep concurrent login's fresh step token on success.

The step record is already consumed by the atomic claim, so deleting
user meta again on success could revoke a fresh challenge issued to a
password login in another browser. Clear only this browser's cookie.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 inc/spbc-auth.php                             | 498 +++++++++-
 .../SpbctWP/G2FA/GoogleAuthenticator.php      |   8 +-
 .../SpbcRateLimit/SpbcUserRateLimiter.php     |  59 ++
 security-malware-firewall.php                 |   3 +
 tests/Inc/TwoFactorAuthTest.php               | 938 ++++++++++++++++++
 .../SpbctWP/G2FA/TestGoogleAuthenticator.php  |  38 +
 .../RateLimiter/TestSpbcUserRateLimiter.php   | 151 +++
 7 files changed, 1641 insertions(+), 54 deletions(-)
 create mode 100644 lib/CleantalkSP/SpbctWP/SpbcRateLimit/SpbcUserRateLimiter.php
 create mode 100644 tests/Inc/TwoFactorAuthTest.php
 create mode 100644 tests/lib/CleantalkSP/SpbctWP/G2FA/TestGoogleAuthenticator.php
 create mode 100644 tests/lib/CleantalkSP/SpbctWP/RateLimiter/TestSpbcUserRateLimiter.php

diff --git a/inc/spbc-auth.php b/inc/spbc-auth.php
index 1c60b3a99..9ecb2e58b 100644
--- a/inc/spbc-auth.php
+++ b/inc/spbc-auth.php
@@ -1,12 +1,14 @@
 <?php
 
 use CleantalkSP\Common\TT;
+use CleantalkSP\Common\TextPlateStatic;
 use CleantalkSP\Common\RateLimit\RateLimiterConfig;
 use CleantalkSP\SpbctWP\Counters\SecurityCounter;
 use CleantalkSP\SpbctWP\Firewall\WafBlocker;
 use CleantalkSP\SpbctWP\G2FA\GoogleAuthenticator;
 use CleantalkSP\SpbctWP\G2FA\QRCode;
 use CleantalkSP\SpbctWP\SpbcRateLimit\SpbcRateLimiter;
+use CleantalkSP\SpbctWP\SpbcRateLimit\SpbcUserRateLimiter;
 use CleantalkSP\SpbctWP\UsersPassCheckModule\UsersPassCheckModel;
 use CleantalkSP\SpbctWP\Variables\Cookie;
 use CleantalkSP\SpbctWP\Helpers\IP;
@@ -50,12 +52,16 @@
 add_filter('manage_users_custom_column', 'spbc_user_last_login_column_content', 10, 3);
 
 if ( isset($spbc) && ($spbc instanceof \CleantalkSP\SpbctWP\State ) && $spbc->settings['2fa__enable'] ) {
-    if (Server::inUriCanonical('wp-login.php') && !empty($_POST)) {
+    // Registered by the POST field, not by URI: the login page could be renamed
+    if (isset($_POST['spbc_2fa'])) {
         add_action('init', 'spbc_2fa_rate_limit', 10, 1);
     }
     add_action('login_form_login', 'spbc_2fa__authenticate', 1);     // Authenticate with Code
     add_action('login_form', 'spbc_2fa__show_field', 10);
     add_action('after_password_reset', 'spbc_2fa__2fa_app_replace_meta', 10, 1);
+    add_action('after_password_reset', 'spbc_2fa__user_rate_limiter_reset', 10, 1);
+    // A challenge issued for the old password must not complete the login
+    add_action('after_password_reset', 'spbc_2fa__challenge_revoke', 10, 1);
     // Profile page hook
     add_action('show_user_profile', 'spbc_2fa__SelfUserProfileEdit', 1);
     add_action('edit_user_profile', 'spbc_2fa__SelfUserProfileEdit', 1);
@@ -74,7 +80,300 @@
  */
 function spbc_2fa__2fa_app_replace_meta(\WP_User $user)
 {
-    return delete_user_meta($user->ID, 'spbc_2fa_type') && delete_user_meta($user->ID, 'spbc_2fa_app_token');
+    delete_user_meta($user->ID, 'spbc_2fa_type');
+    delete_user_meta($user->ID, 'spbc_2fa_app_token');
+    delete_user_meta($user->ID, 'spbc_2fa_last_step');
+
+    return get_user_meta($user->ID, 'spbc_2fa_type', true) === ''
+        && get_user_meta($user->ID, 'spbc_2fa_app_token', true) === '';
+}
+
+/**
+ * Issue a one-time token proving that the first factor (password) has been passed.
+ * Stored hashed in user meta (one record per user, overwritten on each login) and sent to the browser as HttpOnly cookie.
+ *
+ * @param \WP_User $user
+ *
+ * @return void
+ */
+function spbc_2fa__step_token_issue(\WP_User $user)
+{
+    $token   = bin2hex(random_bytes(32));
+    $expires = time() + SPBC_2FA_KEY_TTL;
+
+    update_user_meta(
+        $user->ID,
+        'spbc_2fa_step',
+        array(
+            'hash'    => hash('sha256', $token),
+            'expires' => $expires,
+            'fails'   => 0,
+        )
+    );
+
+    spbc_2fa__step_cookie_set($token, $expires);
+}
+
+/**
+ * @param string $value
+ * @param int    $expires
+ *
+ * @return void
+ */
+function spbc_2fa__step_cookie_set($value, $expires)
+{
+    if ( headers_sent() ) {
+        return;
+    }
+
+    /**
+     * COOKIE_DOMAIN is false by default in WordPress, despite Psalm's string type
+     * @psalm-suppress RedundantCast
+     */
+    $domain = defined('COOKIE_DOMAIN') ? (string) COOKIE_DOMAIN : '';
+
+    \CleantalkSP\Variables\Cookie::set(
+        'spbc_2fa_step',
+        $value,
+        $expires,
+        '/',
+        $domain,
+        is_ssl(),
+        true,
+        'Lax'
+    );
+}
+
+/**
+ * @return string Token from the request or empty string
+ */
+function spbc_2fa__step_token_get_from_request()
+{
+    /**
+     * $_COOKIE value may be an array (spbc_2fa_step[]=...), despite Psalm's string type
+     * @psalm-suppress RedundantCondition
+     */
+    $token = isset($_COOKIE['spbc_2fa_step']) && is_string($_COOKIE['spbc_2fa_step'])
+        ? $_COOKIE['spbc_2fa_step']
+        : '';
+
+    return preg_match('/^[a-f0-9]{64}$/', $token) ? $token : '';
+}
+
+/**
+ * Check that the current request carries a valid 2FA step token for the user.
+ *
+ * @param \WP_User $user
+ *
+ * @return bool
+ */
+function spbc_2fa__step_token_is_valid(\WP_User $user)
+{
+    $token = spbc_2fa__step_token_get_from_request();
+    if ( $token === '' ) {
+        return false;
+    }
+
+    $step = get_user_meta($user->ID, 'spbc_2fa_step', true);
+    if ( ! is_array($step) || empty($step['hash']) || empty($step['expires']) ) {
+        return false;
+    }
+
+    if ( (int) $step['expires'] < time() || (int) $step['fails'] >= SPBC_2FA_MAX_ATTEMPTS ) {
+        delete_user_meta($user->ID, 'spbc_2fa_step');
+        return false;
+    }
+
+    return hash_equals((string) $step['hash'], hash('sha256', $token));
+}
+
+/**
+ * Atomically replace (or delete) the stored step token record, only if it still equals $old.
+ * Of concurrent requests holding the same record exactly one succeeds.
+ *
+ * @param \WP_User   $user
+ * @param array      $old Record read by this request
+ * @param array|null $new New record or null to delete
+ *
+ * @return bool
+ */
+function spbc_2fa__step_token_swap(\WP_User $user, array $old, $new = null)
+{
+    global $wpdb;
+
+    if ( $new === null ) {
+        $result = $wpdb->query(
+            $wpdb->prepare(
+                "DELETE FROM {$wpdb->usermeta} WHERE user_id = %d AND meta_key = %s AND meta_value = %s",
+                $user->ID,
+                'spbc_2fa_step',
+                maybe_serialize($old)
+            )
+        );
+    } else {
+        $result = $wpdb->query(
+            $wpdb->prepare(
+                "UPDATE {$wpdb->usermeta} SET meta_value = %s WHERE user_id = %d AND meta_key = %s AND meta_value = %s",
+                maybe_serialize($new),
+                $user->ID,
+                'spbc_2fa_step',
+                maybe_serialize($old)
+            )
+        );
+    }
+
+    wp_cache_delete($user->ID, 'user_meta');
+
+    return (int) $result > 0;
+}
+
+/**
+ * Atomically count a code attempt before the code check, so parallel requests can't exceed the per-token limit.
+ *
+ * @param \WP_User $user
+ *
+ * @return array|false Reserved record or false if the token was changed by a concurrent request
+ */
+function spbc_2fa__step_token_reserve_attempt(\WP_User $user)
+{
+    $step = get_user_meta($user->ID, 'spbc_2fa_step', true);
+    if ( ! is_array($step) ) {
+        return false;
+    }
+
+    $reserved          = $step;
+    $reserved['fails'] = (int) $step['fails'] + 1;
+
+    return spbc_2fa__step_token_swap($user, $step, $reserved) ? $reserved : false;
+}
+
+/**
+ * Atomically consume the token on the correct code. Only one of concurrent requests may log in.
+ *
+ * @param \WP_User $user
+ * @param array    $reserved Record returned by spbc_2fa__step_token_reserve_attempt()
+ *
+ * @return bool
+ */
+function spbc_2fa__step_token_claim(\WP_User $user, array $reserved)
+{
+    return spbc_2fa__step_token_swap($user, $reserved, null);
+}
+
+/**
+ * Handle a wrong code. Destroys the step token when attempts are exhausted.
+ *
+ * @param \WP_User $user
+ * @param array    $reserved Record returned by spbc_2fa__step_token_reserve_attempt()
+ *
+ * @return bool True if the step token is still usable
+ */
+function spbc_2fa__step_token_after_fail(\WP_User $user, array $reserved)
+{
+    if ( (int) $reserved['fails'] < SPBC_2FA_MAX_ATTEMPTS ) {
+        return true;
+    }
+
+    spbc_2fa__challenge_revoke($user);
+    spbc_2fa__step_cookie_set('', time() - 3600);
+
+    return false;
+}
+
+/**
+ * Revoke the pending 2FA challenge: the step token and the e-mail code.
+ * The password has to be entered again to get a new one.
+ *
+ * @param \WP_User $user
+ *
+ * @return void
+ */
+function spbc_2fa__challenge_revoke(\WP_User $user)
+{
+    global $spbc;
+
+    delete_user_meta($user->ID, 'spbc_2fa_step');
+
+    if ( isset($spbc->data['2fa_keys'][ $user->user_login ]) ) {
+        unset($spbc->data['2fa_keys'][ $user->user_login ]);
+        $spbc->save('data');
+    }
+}
+
+/**
+ * Per-user limiter of 2FA code attempts. Not reset by a new password login and not bound to IP.
+ *
+ * @param \WP_User $user
+ *
+ * @return SpbcUserRateLimiter
+ */
+function spbc_2fa__user_rate_limiter(\WP_User $user)
+{
+    return new SpbcUserRateLimiter(
+        new RateLimiterConfig('2fa_user_attempts', SPBC_2FA_USER_MAX_ATTEMPTS, SPBC_2FA_USER_LOCK_PERIOD),
+        $user->ID
+    );
+}
+
+/**
+ * Unlock 2FA step after the password reset: the user has proved access to the account e-mail.
+ *
+ * @param \WP_User $user
+ *
+ * @return void
+ */
+function spbc_2fa__user_rate_limiter_reset(\WP_User $user)
+{
+    spbc_2fa__user_rate_limiter($user)->reset();
+}
+
+/**
+ * Check 2FA application (TOTP) code with replay protection.
+ *
+ * @param \WP_User $user
+ * @param string   $secret
+ * @param string   $code
+ * @param bool     $is_reused Set to true if the code is correct but its time step was already used
+ * @param bool     $mark_used Mark the time step as used right away. Pass false to call spbc_2fa__app_code_mark_used()
+ *                            later, after the step token is claimed
+ *
+ * @return bool
+ */
+function spbc_2fa__app_code_check(\WP_User $user, $secret, $code, &$is_reused = false, $mark_used = true)
+{
+    $ga        = new GoogleAuthenticator();
+    $time_step = (int) floor(time() / 30);
+    $is_reused = false;
+
+    if ( ! hash_equals((string) $ga->getCode($secret, $time_step), (string) $code) ) {
+        return false;
+    }
+
+    // Reject the code if it (or a newer one) was already used
+    $last_step = (int) get_user_meta($user->ID, 'spbc_2fa_last_step', true);
+    if ( $time_step <= $last_step ) {
+        $is_reused = true;
+        return false;
+    }
+
+    if ( $mark_used ) {
+        spbc_2fa__app_code_mark_used($user);
+    }
+
+    return true;
+}
+
+/**
+ * Mark the current time step as used. If the step has changed since the check, the newer one is marked,
+ * which rejects the checked code as well.
+ *
+ * @param \WP_User $user
+ *
+ * @return void
+ */
+function spbc_2fa__app_code_mark_used(\WP_User $user)
+{
+    update_user_meta($user->ID, 'spbc_2fa_last_step', (int) floor(time() / 30));
 }
 
 /**
@@ -223,6 +522,7 @@ function spbc_authenticate($user, $username)
             if ( $type2fa !== '2fa_application' ) {
                 spbc_2fa__send_mail($user);
             }
+            spbc_2fa__step_token_issue($user);
             $woocommerce_additional_query_string = '';
             if (
                     spbc_is_plugin_active('woocommerce/woocommerce.php') &&
@@ -867,7 +1167,8 @@ function spbc_2fa__show_field()
         if ( $spbc_2fa_user ) {
             $user = spbc_get_user_by('login', rawurldecode($spbc_2fa_user));
 
-            if ( ! $user ) {
+            // The code form is shown only to the one who has just passed the password check
+            if ( ! $user || ! spbc_2fa__step_token_is_valid($user) ) {
                 return;
             }
 
@@ -887,7 +1188,30 @@ function spbc_2fa__show_field()
             }
 
             // Error displaying
-            if ( isset($_GET['spbc_2fa_error']) ) {
+            if ( Get::getString('spbc_2fa_error') === '2' && $type2fa === '2fa_application' ) {
+                // Correct code, but its time step is already used
+                $json_flags = JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT;
+                echo TextPlateStatic::render(
+                    '
+<script>
+    (function () {
+        const spbc_err = document.createElement("div");
+        spbc_err.id = "login_error";
+        const spbc_err_title = document.createElement("strong");
+        spbc_err_title.textContent = {{title}};
+        spbc_err.appendChild(spbc_err_title);
+        spbc_err.appendChild(document.createTextNode({{text}}));
+        const spbc_login = document.getElementById("login");
+        spbc_login.insertBefore(spbc_err, spbc_login.children[1]);
+    })();
+</script>
+                    ',
+                    array(
+                        'title' => wp_json_encode(__('CODE ALREADY USED: ', 'security-malware-firewall'), $json_flags),
+                        'text'  => wp_json_encode(__('Please, wait for the next code in your 2FA Code application and try again in 30 seconds.', 'security-malware-firewall'), $json_flags),
+                    )
+                );
+            } elseif ( isset($_GET['spbc_2fa_error']) ) {
                 echo '<script>'
                      . 'var spbc_err = document.createElement("div");'
                      . 'spbc_err.innerHTML = \'<div id="login_error">'
@@ -958,21 +1282,7 @@ function spbc_2fa__key_generate_and_store($user)
     global $spbc;
 
     // Generate key
-    $left_part_for_pad = hexdec(substr(hash('sha256', $spbc->data['salt'] . rand(0, getrandmax())), 0, 6)) % 1000;
-    $right_part_for_pad = hexdec(substr(hash('sha256', $spbc->data['salt'] . rand(0, getrandmax())), 0, 6)) % 1000;
-    $left_part  = str_pad(
-        (string) $left_part_for_pad,
-        3,
-        '1',
-        STR_PAD_LEFT
-    );
-    $right_part = str_pad(
-        (string) $right_part_for_pad,
-        3,
-        '0',
-        STR_PAD_LEFT
-    );
-    $key        = $left_part . $right_part;
+    $key = (string) random_int(100000, 999999);
 
     $spbc->data['2fa_keys'][ $user->user_login ] = array(
         'generated' => time(),
@@ -1027,33 +1337,78 @@ function spbc_2fa__authenticate()
 
     if ( $spbc->settings['2fa__enable'] ) {
         if ( isset($_POST['spbc_2fa'], $_POST['log']) ) {
-            $user = get_user_by('login', $_POST['log']);
+            $login = is_string($_POST['log']) ? wp_unslash($_POST['log']) : '';
+            $code  = is_string($_POST['spbc_2fa']) ? trim(wp_unslash($_POST['spbc_2fa'])) : '';
+
+            $user = get_user_by('login', $login);
 
             if (!$user) {
-                $user = get_user_by('email', $_POST['log']);
+                $user = get_user_by('email', $login);
             }
 
             if ( $user instanceof WP_User ) {
+                // The code is accepted only as a continuation of the successful password check
+                if (
+                    ! spbc_is_user_role_in($spbc->settings['2fa__roles'], $user) ||
+                    ! spbc_2fa__step_token_is_valid($user)
+                ) {
+                    spbc_2fa__failed($user, true);
+                }
+
+                // Every attempt is counted atomically before the code check, so parallel requests can't exceed the limit
+                $user_rate_limiter = spbc_2fa__user_rate_limiter($user);
+                if ( ! $user_rate_limiter->checkPassed() ) {
+                    spbc_2fa__locked($user);
+                }
+
+                // Per-token attempt is reserved atomically too; a concurrent request holding the same token loses
+                $step = spbc_2fa__step_token_reserve_attempt($user);
+                if ( ! $step ) {
+                    spbc_2fa__failed($user, true);
+                }
+
                 spbc_2fa__key_remove_old();
 
-                $type2fa    = get_user_meta($user->ID, 'spbc_2fa_type', true);
+                $type2fa        = get_user_meta($user->ID, 'spbc_2fa_type', true);
                 $_2fa_app_token = get_user_meta($user->ID, 'spbc_2fa_app_token', true);
-                $ga         = new GoogleAuthenticator();
-
-                // 2fa passed
-                if ( isset($spbc->data['2fa_keys'][ $user->user_login ]) && $spbc->data['2fa_keys'][ $user->user_login ]['val'] == $_POST['spbc_2fa'] ) {
-                    // Delete it so no one could login via this code
-                    unset($spbc->data['2fa_keys'][ $user->user_login ]);
-                    $spbc->save('data');
-
-                    spbc_authenticate__write_log_login($user);
-                    spbc_2fa__success($user);
-                } elseif ( $type2fa === '2fa_application' && $_2fa_app_token && $_POST['spbc_2fa'] == $ga->getCode($_2fa_app_token) ) {
-                    spbc_authenticate__write_log_login($user);
-                    spbc_2fa__success($user);
-                } else {
-                    spbc_2fa__failed($user);
+                $is_code_reused = false;
+
+                if ( preg_match('/^\d{6}$/', $code) ) {
+                    // 2fa passed
+                    if (
+                        isset($spbc->data['2fa_keys'][ $user->user_login ]) &&
+                        hash_equals((string) $spbc->data['2fa_keys'][ $user->user_login ]['val'], $code)
+                    ) {
+                        // Only one of concurrent requests with the same token may log in
+                        if ( ! spbc_2fa__step_token_claim($user, $step) ) {
+                            spbc_2fa__failed($user, true);
+                        }
+
+                        // Delete it so no one could login via this code
+                        unset($spbc->data['2fa_keys'][ $user->user_login ]);
+                        $spbc->save('data');
+
+                        $user_rate_limiter->reset();
+                        spbc_authenticate__write_log_login($user);
+                        spbc_2fa__success($user);
+                    } elseif (
+                        $type2fa === '2fa_application' &&
+                        $_2fa_app_token &&
+                        spbc_2fa__app_code_check($user, $_2fa_app_token, $code, $is_code_reused, false)
+                    ) {
+                        // Only one of concurrent requests with the same token may log in
+                        if ( ! spbc_2fa__step_token_claim($user, $step) ) {
+                            spbc_2fa__failed($user, true);
+                        }
+
+                        spbc_2fa__app_code_mark_used($user);
+                        $user_rate_limiter->reset();
+                        spbc_authenticate__write_log_login($user);
+                        spbc_2fa__success($user);
+                    }
                 }
+
+                spbc_2fa__failed($user, ! spbc_2fa__step_token_after_fail($user, $step), $is_code_reused);
             }
         }
     }
@@ -1063,6 +1418,10 @@ function spbc_2fa__success(\WP_User $user)
 {
     global $spbc;
 
+    // The step record is already consumed by spbc_2fa__step_token_claim(). Don't touch user meta here:
+    // it may already hold a fresh token of a concurrent password login in another browser.
+    spbc_2fa__step_cookie_set('', time() - 3600);
+
     // Check if password is leaked after 2FA success
     if (UsersPassCheckHandler::isUserPassLeaked($user->ID)) {
         // Set force password change flag
@@ -1129,7 +1488,42 @@ function spbc_2fa__success(\WP_User $user)
     die();
 }
 
-function spbc_2fa__failed(\WP_User $user)
+/**
+ * 2FA step is locked for the user because of too many wrong codes.
+ * Reachable only with a valid step token, so it can't be triggered anonymously.
+ *
+ * @param \WP_User $user
+ */
+function spbc_2fa__locked(\WP_User $user)
+{
+    $type2fa = get_user_meta($user->ID, 'spbc_2fa_type', true);
+
+    spbc_auth_log(
+        array(
+            'username'     => $user->user_login,
+            'event'        => $type2fa === '2fa_application' ? 'auth_failed_g2fa' : 'auth_failed_2fa',
+            'roles'        => reset($user->roles),
+            'user_agent'   => htmlspecialchars(Server::getString('HTTP_USER_AGENT')),
+            'browser_sign' => spbc_authenticate__browser_sign__get_hash($user),
+        )
+    );
+
+    spbc_2fa__challenge_revoke($user);
+    spbc_2fa__step_cookie_set('', time() - 3600);
+
+    wp_die(
+        esc_html__('Too many wrong authorization codes. Please try again later. Protected - Security by CleanTalk.', 'security-malware-firewall'),
+        'Forbidden',
+        array('response' => 403)
+    );
+}
+
+/**
+ * @param \WP_User $user
+ * @param bool     $restart Send the user back to the password form (no valid 2FA step token or attempts exhausted)
+ * @param bool     $is_code_reused The code is correct, but was already used - the user has to wait for the next one
+ */
+function spbc_2fa__failed(\WP_User $user, $restart = false, $is_code_reused = false)
 {
     $type2fa = get_user_meta($user->ID, 'spbc_2fa_type', true);
     $event   = $type2fa === '2fa_application' ? 'auth_failed_g2fa' : 'auth_failed_2fa';
@@ -1143,10 +1537,18 @@ function spbc_2fa__failed(\WP_User $user)
         'browser_sign' => spbc_authenticate__browser_sign__get_hash($user),
         )
     );
+
+    if ( $restart ) {
+        // Only the requester's cookie is cleared: server-side state must not be touched by a request without a valid token
+        spbc_2fa__step_cookie_set('', time() - 3600);
+        wp_redirect(wp_login_url());
+        die();
+    }
+
     wp_redirect(
         wp_login_url()
         . ( strpos(wp_login_url(), '?') === false ? '?' : '&' )
-        . 'spbc_2fa_error=1&spbc_2fa_user=' . $user->user_login
+        . 'spbc_2fa_error=' . ( $is_code_reused ? '2' : '1' ) . '&spbc_2fa_user=' . rawurlencode($user->user_login)
     );
     die();
 }
@@ -1364,13 +1766,16 @@ function spbc_2fa__Get2FAAppQrCode()
     }
 
     if (property_exists($user_obj, 'ID')) {
-        $user_token = get_user_meta($user_obj->ID, 'spbc_2fa_app_token', true);
+        // The secret is never exposed again once 2FA application is activated
+        if (get_user_meta($user_obj->ID, 'spbc_2fa_type', true) === '2fa_application') {
+            wp_send_json_error(esc_html__('Error: 2FA application is already enabled. Disable it first to set up a new one.', 'security-malware-firewall'));
+        }
+
         $ga = new GoogleAuthenticator();
 
-        if (!$user_token) {
-            $user_token = $ga->generateSecret();
-            update_user_meta($user_obj->ID, 'spbc_2fa_app_token', $user_token);
-        }
+        // Fresh secret on each setup attempt, so a secret leaked from an abandoned setup is not reused
+        $user_token = $ga->generateSecret();
+        update_user_meta($user_obj->ID, 'spbc_2fa_app_token', $user_token);
 
         $otpauth = $ga->getUrl(
             $user_obj->user_login,
@@ -1409,10 +1814,7 @@ function spbc_2fa__Check2FAAppCode()
             $user_token = get_user_meta($user_obj->ID, 'spbc_2fa_app_token', true);
 
             if ($user_token) {
-                $ga   = new GoogleAuthenticator();
-                $code = $ga->getCode($user_token);
-
-                if (trim($_POST['code']) === $code) {
+                if (spbc_2fa__app_code_check($user_obj, $user_token, trim($_POST['code']))) {
                     if (update_user_meta($user_obj->ID, 'spbc_2fa_type', '2fa_application')) {
                         wp_send_json_success();
                     }
diff --git a/lib/CleantalkSP/SpbctWP/G2FA/GoogleAuthenticator.php b/lib/CleantalkSP/SpbctWP/G2FA/GoogleAuthenticator.php
index ea8addd09..1b7c238b6 100644
--- a/lib/CleantalkSP/SpbctWP/G2FA/GoogleAuthenticator.php
+++ b/lib/CleantalkSP/SpbctWP/G2FA/GoogleAuthenticator.php
@@ -18,7 +18,7 @@ class GoogleAuthenticator
 {
     public static $PASS_CODE_LENGTH = 6;
     public static $PIN_MODULO;
-    public static $SECRET_LENGTH = 10;
+    public static $SECRET_LENGTH = 20;
 
     public function __construct()
     {
@@ -84,11 +84,7 @@ public function getUrl($user, $hostname, $secret)
 
     public function generateSecret()
     {
-        $secret = "";
-        for ( $i = 1; $i <= self::$SECRET_LENGTH; $i++ ) {
-            $c      = rand(0, 255);
-            $secret .= pack("c", $c);
-        }
+        $secret = random_bytes(self::$SECRET_LENGTH);
         $base32 = new FixedBitNotation(5, 'ABCDEFGHIJKLMNOPQRSTUVWXYZ234567', true, true);
 
         return $base32->encode($secret);
diff --git a/lib/CleantalkSP/SpbctWP/SpbcRateLimit/SpbcUserRateLimiter.php b/lib/CleantalkSP/SpbctWP/SpbcRateLimit/SpbcUserRateLimiter.php
new file mode 100644
index 000000000..99c06835b
--- /dev/null
+++ b/lib/CleantalkSP/SpbctWP/SpbcRateLimit/SpbcUserRateLimiter.php
@@ -0,0 +1,59 @@
+<?php
+
+namespace CleantalkSP\SpbctWP\SpbcRateLimit;
+
+use CleantalkSP\Common\RateLimit\RateLimiterConfig;
+
+/**
+ * Rate limiter keyed by WordPress user ID instead of IP address.
+ * Changing IP does not reset the counter.
+ *
+ * @package CleantalkSP\SpbctWP\SpbcRateLimit
+ */
+class SpbcUserRateLimiter extends SpbcRateLimiter
+{
+    /**
+     * @var int
+     */
+    private $user_id;
+
+    /**
+     * @param RateLimiterConfig $config
+     * @param int               $user_id
+     */
+    public function __construct(RateLimiterConfig $config, $user_id)
+    {
+        $this->user_id = (int) $user_id;
+        parent::__construct($config);
+    }
+
+    protected function setUID(): void
+    {
+        $this->uid = md5('user_' . $this->user_id . $this->config->type);
+    }
+
+    /**
+     * UA is informational here (the limiter is keyed by user ID), but an attacker-controlled value
+     * must not break the insert: the limiter fails open on DB errors, and wpdb rejects queries with
+     * text invalid for the table charset. Printable ASCII fits any charset and is truncated safely.
+     */
+    protected function setUA(): void
+    {
+        parent::setUA();
+        $this->ua = substr((string) preg_replace('/[^\x20-\x7E]/', '', $this->ua), 0, 200);
+    }
+
+    /**
+     * Drop the counter, e.g. after a successful attempt.
+     *
+     * @return void
+     */
+    public function reset(): void
+    {
+        global $wpdb;
+
+        $wpdb->query(
+            $wpdb->prepare('DELETE FROM ' . SPBC_TBL_RATE_LIMITS . ' WHERE uid = %s', $this->uid)
+        );
+    }
+}
diff --git a/security-malware-firewall.php b/security-malware-firewall.php
index 8010142d2..8f66b2b3a 100644
--- a/security-malware-firewall.php
+++ b/security-malware-firewall.php
@@ -101,6 +101,9 @@
 
 // Auth params
 define('SPBC_2FA_KEY_TTL', 600);      // 2fa key lifetime in seconds
+define('SPBC_2FA_MAX_ATTEMPTS', 5);   // 2fa wrong code attempts allowed before the password has to be entered again
+define('SPBC_2FA_USER_MAX_ATTEMPTS', 10);  // 2fa code attempts per user allowed within SPBC_2FA_USER_LOCK_PERIOD (reset on success)
+define('SPBC_2FA_USER_LOCK_PERIOD', 900);  // 2fa per user counting window and lock time in seconds
 
 /**
  * Set SPBC_BFP_DISABLE to true to disable Brute Force Protection module.
diff --git a/tests/Inc/TwoFactorAuthTest.php b/tests/Inc/TwoFactorAuthTest.php
new file mode 100644
index 000000000..bc69cca7b
--- /dev/null
+++ b/tests/Inc/TwoFactorAuthTest.php
@@ -0,0 +1,938 @@
+<?php
+
+namespace Inc;
+
+use CleantalkSP\SpbctWP\DB\TablesCreator;
+use CleantalkSP\SpbctWP\G2FA\GoogleAuthenticator;
+use CleantalkSP\Variables\Get;
+use CleantalkSP\Variables\Server;
+use PHPUnit\Framework\AssertionFailedError;
+
+/**
+ * Thrown from the hooks to stop the flow where production code would send headers and die.
+ */
+class TwoFactorAuthTestInterrupt extends \Exception
+{
+    public $type;
+    public $value;
+
+    public function __construct($type, $value)
+    {
+        parent::__construct($type);
+        $this->type  = $type;
+        $this->value = $value;
+    }
+}
+
+/**
+ * 2FA step: binding to the first factor, replay protection, per-user attempts limit, setup endpoints.
+ */
+class TwoFactorAuthTest extends \SpbcTestCase
+{
+    const SECRET = 'GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ';
+
+    /** @var \WP_User */
+    private static $app_user_static;
+    /** @var \WP_User */
+    private static $email_user_static;
+    /** @var \WP_User */
+    private static $subscriber_static;
+
+    private $app_user;
+    private $email_user;
+    private $subscriber;
+    private $saved_server;
+    private $parallel_hook;
+    private $parallel_results = array();
+
+    /**
+     * Users are created once per class: wp_insert_user()/wp_delete_user() are the slowest part of the test.
+     */
+    public static function setUpBeforeClass(): void
+    {
+        global $wpdb;
+
+        parent::setUpBeforeClass();
+
+        $tables_creator = new TablesCreator();
+        $tables_creator->createTable(SPBC_TBL_SECURITY_LOG);
+        $tables_creator->createTable(SPBC_TBL_RATE_LIMITS);
+        $tables_creator->createTable($wpdb->base_prefix . 'spbc_users_pass');
+
+        self::$app_user_static   = self::createUser('spbc_2fa_app', 'editor');
+        self::$email_user_static = self::createUser('spbc_2fa_mail', 'editor');
+        self::$subscriber_static = self::createUser('spbc_2fa_sub', 'subscriber');
+    }
+
+    public static function tearDownAfterClass(): void
+    {
+        require_once ABSPATH . 'wp-admin/includes/user.php';
+        foreach ( array(self::$app_user_static, self::$email_user_static, self::$subscriber_static) as $user ) {
+            wp_delete_user($user->ID);
+        }
+
+        parent::tearDownAfterClass();
+    }
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->app_user   = self::$app_user_static;
+        $this->email_user = self::$email_user_static;
+        $this->subscriber = self::$subscriber_static;
+
+        $this->saved_server = $_SERVER;
+        $this->resetState();
+
+        add_filter('wp_redirect', array($this, 'interruptRedirect'), 1);
+        add_filter('wp_die_handler', array($this, 'getDieHandler'), PHP_INT_MAX);
+        add_filter('wp_die_ajax_handler', array($this, 'getDieHandler'), PHP_INT_MAX);
+        add_action('set_auth_cookie', array($this, 'interruptAuthCookie'), 1, 4);
+    }
+
+    protected function tearDown(): void
+    {
+        remove_filter('wp_redirect', array($this, 'interruptRedirect'), 1);
+        remove_filter('wp_die_handler', array($this, 'getDieHandler'), PHP_INT_MAX);
+        remove_filter('wp_die_ajax_handler', array($this, 'getDieHandler'), PHP_INT_MAX);
+        remove_action('set_auth_cookie', array($this, 'interruptAuthCookie'), 1);
+
+        $this->resetState();
+        $_SERVER = $this->saved_server;
+        $this->resetRequestCache();
+
+        parent::tearDown();
+    }
+
+    /**
+     * Brings settings, request, users' 2FA meta and limiter counters to the initial state.
+     */
+    private function resetState()
+    {
+        global $spbc, $wpdb;
+
+        $spbc->settings['2fa__enable'] = 1;
+        $spbc->settings['2fa__roles']  = array('editor');
+        $spbc->data['2fa_keys']        = array();
+
+        $_POST = $_GET = $_COOKIE = $_REQUEST = array();
+        $_SERVER['REMOTE_ADDR']     = '10.10.10.10';
+        $_SERVER['HTTP_USER_AGENT'] = 'PHPUnit';
+        $this->resetRequestCache();
+
+        remove_filter('wp_doing_ajax', '__return_true');
+        if ( $this->parallel_hook ) {
+            remove_filter('query', $this->parallel_hook);
+            $this->parallel_hook = null;
+        }
+        $this->parallel_results = array();
+        wp_set_current_user(0);
+
+        $wpdb->query('DELETE FROM ' . SPBC_TBL_RATE_LIMITS);
+
+        foreach ( array($this->app_user, $this->email_user, $this->subscriber) as $user ) {
+            foreach ( array('spbc_2fa_type', 'spbc_2fa_app_token', 'spbc_2fa_step', 'spbc_2fa_last_step') as $meta ) {
+                delete_user_meta($user->ID, $meta);
+            }
+        }
+        update_user_meta($this->app_user->ID, 'spbc_2fa_type', '2fa_application');
+        update_user_meta($this->app_user->ID, 'spbc_2fa_app_token', self::SECRET);
+    }
+
+    // ---------------------------------------------------------------- hooks
+
+    public function interruptRedirect($location)
+    {
+        throw new TwoFactorAuthTestInterrupt('redirect', $location);
+    }
+
+    public function getDieHandler()
+    {
+        return array($this, 'interruptDie');
+    }
+
+    public function interruptDie($message, $title = '', $args = array())
+    {
+        $code = is_array($args) && isset($args['response']) ? $args['response'] : $args;
+        throw new TwoFactorAuthTestInterrupt('die', (int) $code);
+    }
+
+    public function interruptAuthCookie($auth_cookie, $expire, $expiration, $user_id)
+    {
+        throw new TwoFactorAuthTestInterrupt('auth', (int) $user_id);
+    }
+
+    // ---------------------------------------------------------------- helpers
+
+    private function resetRequestCache()
+    {
+        Server::getInstance()->variables = array();
+        Get::getInstance()->variables    = array();
+        \CleantalkSP\SpbctWP\Helpers\IP::getInstance()->ips_stored = array();
+    }
+
+    private static function createUser($login, $role)
+    {
+        $existing = get_user_by('login', $login);
+        if ( $existing ) {
+            require_once ABSPATH . 'wp-admin/includes/user.php';
+            wp_delete_user($existing->ID);
+        }
+
+        $user_id = wp_insert_user(array(
+            'user_login' => $login,
+            'user_pass'  => wp_generate_password(),
+            'user_email' => $login . '@example.com',
+            'role'       => $role,
+        ));
+
+        return get_user_by('id', $user_id);
+    }
+
+    /**
+     * Emulates the token issued after a successful password check.
+     */
+    private function giveStepToken(\WP_User $user, $fails = 0, $expires = null)
+    {
+        $token = bin2hex(random_bytes(32));
+        update_user_meta($user->ID, 'spbc_2fa_step', array(
+            'hash'    => hash('sha256', $token),
+            'expires' => $expires === null ? time() + 600 : $expires,
+            'fails'   => $fails,
+        ));
+        $_COOKIE['spbc_2fa_step'] = $token;
+
+        return $token;
+    }
+
+    private function timeStep()
+    {
+        return (int) floor(time() / 30);
+    }
+
+    private function codeForStep($time_step)
+    {
+        $ga = new GoogleAuthenticator();
+
+        return $ga->getCode(self::SECRET, $time_step);
+    }
+
+    private function currentCode()
+    {
+        return $this->codeForStep($this->timeStep());
+    }
+
+    /**
+     * A code that is invalid now and stays invalid if the time step changes during the test.
+     */
+    private function wrongCode()
+    {
+        $step  = $this->timeStep();
+        $valid = array($this->codeForStep($step - 1), $this->codeForStep($step), $this->codeForStep($step + 1));
+
+        for ( $code = 0; in_array(sprintf('%06d', $code), $valid, true); $code++ ) {
+        }
+
+        return sprintf('%06d', $code);
+    }
+
+    /**
+     * Runs a scenario which needs a valid TOTP code. Instead of waiting for a fresh time step,
+     * the scenario is repeated from the clean state if it failed because the step changed in the middle.
+     *
+     * @param callable $scenario Receives the code of the current time step
+     */
+    private function inOneTimeStep(callable $scenario)
+    {
+        for ( $attempt = 1;; $attempt++ ) {
+            $step = $this->timeStep();
+            try {
+                $scenario($this->codeForStep($step));
+                return;
+            } catch ( AssertionFailedError $e ) {
+                if ( $attempt >= 2 || $this->timeStep() === $step ) {
+                    throw $e;
+                }
+                $this->resetState();
+            }
+        }
+    }
+
+    /**
+     * Emulates a concurrent request: runs $parallel right before the first query matching $pattern is executed.
+     */
+    private function runParallelBeforeQuery($pattern, callable $parallel)
+    {
+        $triggered           = false;
+        $this->parallel_hook = function ($query) use ($pattern, $parallel, &$triggered) {
+            if ( ! $triggered && preg_match($pattern, $query) ) {
+                $triggered                = true;
+                $this->parallel_results[] = $parallel();
+            }
+            return $query;
+        };
+        add_filter('query', $this->parallel_hook);
+    }
+
+    /**
+     * @return array [type, value]: 'auth' => user id, 'redirect' => location, 'die' => status code, 'none' => null
+     */
+    private function postCode($login, $code)
+    {
+        $_POST['log']      = $login;
+        $_POST['spbc_2fa'] = $code;
+
+        return $this->runInterruptible('spbc_2fa__authenticate');
+    }
+
+    private function runInterruptible($callback)
+    {
+        $args = array_slice(func_get_args(), 1);
+        try {
+            call_user_func_array($callback, $args);
+        } catch ( TwoFactorAuthTestInterrupt $e ) {
+            return array($e->type, $e->value);
+        }
+
+        return array('none', null);
+    }
+
+    private function assertRestart(array $result)
+    {
+        $this->assertSame(array('redirect', wp_login_url()), $result);
+    }
+
+    private function assertCodeError(array $result, $error, \WP_User $user)
+    {
+        $this->assertSame('redirect', $result[0]);
+        $this->assertStringContainsString('spbc_2fa_error=' . $error, $result[1]);
+        $this->assertStringContainsString('spbc_2fa_user=' . rawurlencode($user->user_login), $result[1]);
+    }
+
+    private function captureOutput($callback)
+    {
+        ob_start();
+        try {
+            $result = $this->runInterruptible($callback);
+        } finally {
+            $output = ob_get_clean();
+        }
+
+        return array($result, $output);
+    }
+
+    private function asAjaxUser(\WP_User $user)
+    {
+        wp_set_current_user($user->ID);
+        add_filter('wp_doing_ajax', '__return_true');
+        $_REQUEST['security'] = wp_create_nonce('ct_secret_stuff');
+    }
+
+    // ---------------------------------------------------------------- step token
+
+    public function testStepTokenIssueStoresOnlyHash()
+    {
+        spbc_2fa__step_token_issue($this->app_user);
+
+        $step = get_user_meta($this->app_user->ID, 'spbc_2fa_step', true);
+        $this->assertRegExp('/^[a-f0-9]{64}$/', $step['hash']);
+        $this->assertSame(0, $step['fails']);
+        $this->assertGreaterThan(time(), $step['expires']);
+    }
+
+    public function testStepTokenValidOnlyWithMatchingCookie()
+    {
+        $this->assertFalse(spbc_2fa__step_token_is_valid($this->app_user), 'no cookie');
+
+        $this->giveStepToken($this->app_user);
+        $this->assertTrue(spbc_2fa__step_token_is_valid($this->app_user));
+
+        $_COOKIE['spbc_2fa_step'] = str_repeat('a', 64);
+        $this->assertFalse(spbc_2fa__step_token_is_valid($this->app_user), 'foreign token');
+
+        $_COOKIE['spbc_2fa_step'] = array('x');
+        $this->assertFalse(spbc_2fa__step_token_is_valid($this->app_user), 'array cookie');
+
+        $_COOKIE['spbc_2fa_step'] = 'not-a-token';
+        $this->assertFalse(spbc_2fa__step_token_is_valid($this->app_user), 'malformed cookie');
+    }
+
+    public function testStepTokenIsBoundToUser()
+    {
+        $this->giveStepToken($this->email_user);
+
+        $this->assertFalse(spbc_2fa__step_token_is_valid($this->app_user));
+    }
+
+    public function testExpiredStepTokenIsRejectedAndDeleted()
+    {
+        $this->giveStepToken($this->app_user, 0, time() - 1);
+
+        $this->assertFalse(spbc_2fa__step_token_is_valid($this->app_user));
+        $this->assertSame('', get_user_meta($this->app_user->ID, 'spbc_2fa_step', true));
+    }
+
+    public function testExhaustedStepTokenIsRejectedAndDeleted()
+    {
+        $this->giveStepToken($this->app_user, SPBC_2FA_MAX_ATTEMPTS);
+
+        $this->assertFalse(spbc_2fa__step_token_is_valid($this->app_user));
+        $this->assertSame('', get_user_meta($this->app_user->ID, 'spbc_2fa_step', true));
+    }
+
+    public function testReserveAttemptCountsFailsAndAfterFailDestroysTokenAndEmailKeyOnLastAttempt()
+    {
+        global $spbc;
+
+        $this->giveStepToken($this->email_user);
+        $spbc->data['2fa_keys'][$this->email_user->user_login] = array('generated' => time(), 'val' => '123456');
+
+        for ( $i = 1; $i < SPBC_2FA_MAX_ATTEMPTS; $i++ ) {
+            $reserved = spbc_2fa__step_token_reserve_attempt($this->email_user);
+            $this->assertSame($i, $reserved['fails']);
+            $this->assertSame($reserved, get_user_meta($this->email_user->ID, 'spbc_2fa_step', true));
+            $this->assertTrue(spbc_2fa__step_token_after_fail($this->email_user, $reserved));
+        }
+
+        $reserved = spbc_2fa__step_token_reserve_attempt($this->email_user);
+        $this->assertFalse(spbc_2fa__step_token_after_fail($this->email_user, $reserved));
+        $this->assertSame('', get_user_meta($this->email_user->ID, 'spbc_2fa_step', true));
+        $this->assertArrayNotHasKey($this->email_user->user_login, $spbc->data['2fa_keys']);
+    }
+
+    public function testReserveAttemptWithoutTokenReturnsFalse()
+    {
+        $this->assertFalse(spbc_2fa__step_token_reserve_attempt($this->app_user));
+    }
+
+    /**
+     * Two requests which read the same record: only the first swap succeeds.
+     */
+    public function testSwapSucceedsOnlyForActualRecord()
+    {
+        $this->giveStepToken($this->app_user);
+        $read_by_both = get_user_meta($this->app_user->ID, 'spbc_2fa_step', true);
+
+        $first = array_merge($read_by_both, array('fails' => 1));
+        $this->assertTrue(spbc_2fa__step_token_swap($this->app_user, $read_by_both, $first));
+        $this->assertFalse(spbc_2fa__step_token_swap($this->app_user, $read_by_both, array_merge($read_by_both, array('fails' => 7))));
+        $this->assertSame($first, get_user_meta($this->app_user->ID, 'spbc_2fa_step', true));
+
+        $this->assertFalse(spbc_2fa__step_token_claim($this->app_user, $read_by_both));
+        $this->assertTrue(spbc_2fa__step_token_claim($this->app_user, $first));
+        $this->assertFalse(spbc_2fa__step_token_claim($this->app_user, $first), 'token can be claimed only once');
+        $this->assertSame('', get_user_meta($this->app_user->ID, 'spbc_2fa_step', true));
+    }
+
+    // ---------------------------------------------------------------- TOTP check
+
+    public function testAppCodeCheckAcceptsCurrentCodeOnce()
+    {
+        $this->inOneTimeStep(function ($code) {
+            $is_reused = null;
+            $this->assertTrue(spbc_2fa__app_code_check($this->app_user, self::SECRET, $code, $is_reused));
+            $this->assertFalse($is_reused);
+            $this->assertSame(
+                $this->timeStep(),
+                (int) get_user_meta($this->app_user->ID, 'spbc_2fa_last_step', true)
+            );
+
+            $this->assertFalse(spbc_2fa__app_code_check($this->app_user, self::SECRET, $code, $is_reused));
+            $this->assertTrue($is_reused, 'Correct code of an already used step must be reported as reused');
+        });
+    }
+
+    public function testAppCodeCheckRejectsWrongAndPreviousStepCode()
+    {
+        $code = $this->currentCode();
+        $ga   = new GoogleAuthenticator();
+
+        $is_reused = null;
+        $this->assertFalse(spbc_2fa__app_code_check($this->app_user, self::SECRET, $this->wrongCode(), $is_reused));
+        $this->assertFalse($is_reused);
+
+        $previous = $ga->getCode(self::SECRET, $this->timeStep() - 1);
+        if ( $previous !== $code ) {
+            $this->assertFalse(spbc_2fa__app_code_check($this->app_user, self::SECRET, $previous));
+        }
+
+        $this->assertSame('', get_user_meta($this->app_user->ID, 'spbc_2fa_last_step', true));
+    }
+
+    // ---------------------------------------------------------------- spbc_2fa__authenticate
+
+    /**
+     * S1 regression: login + valid TOTP without passing the password step must not log in.
+     */
+    public function testTotpWithoutPasswordStepDoesNotLogIn()
+    {
+        $result = $this->postCode($this->app_user->user_login, $this->currentCode());
+
+        $this->assertRestart($result);
+        $this->assertSame('', get_user_meta($this->app_user->ID, 'spbc_2fa_last_step', true));
+    }
+
+    public function testTotpWithTokenOfAnotherUserDoesNotLogIn()
+    {
+        $this->giveStepToken($this->email_user);
+
+        $this->assertRestart($this->postCode($this->app_user->user_login, $this->currentCode()));
+    }
+
+    public function testInvalidTokenDoesNotTouchVictimState()
+    {
+        $this->giveStepToken($this->app_user);
+        $victim_step = get_user_meta($this->app_user->ID, 'spbc_2fa_step', true);
+
+        $_COOKIE['spbc_2fa_step'] = str_repeat('b', 64);
+        for ( $i = 0; $i < SPBC_2FA_MAX_ATTEMPTS + 1; $i++ ) {
+            $this->assertRestart($this->postCode($this->app_user->user_login, '000000'));
+        }
+
+        $this->assertSame($victim_step, get_user_meta($this->app_user->ID, 'spbc_2fa_step', true));
+        $this->assertTrue(spbc_2fa__user_rate_limiter($this->app_user)->checkPassed(), 'limiter must not be spent');
+    }
+
+    public function testValidTotpWithStepTokenLogsIn()
+    {
+        $this->inOneTimeStep(function ($code) {
+            $this->giveStepToken($this->app_user);
+
+            $result = $this->postCode($this->app_user->user_login, $code);
+
+            $this->assertSame(array('auth', $this->app_user->ID), $result);
+            $this->assertSame('', get_user_meta($this->app_user->ID, 'spbc_2fa_step', true), 'token is one-time');
+        });
+    }
+
+    /**
+     * Successful login must not revoke a fresh challenge issued meanwhile to a password login in another browser.
+     */
+    public function testSuccessKeepsFreshTokenOfConcurrentLogin()
+    {
+        $this->inOneTimeStep(function ($code) {
+            $this->giveStepToken($this->app_user);
+            $login = $this->app_user->user_login;
+            $fresh = null;
+
+            // Another browser passes the password right after this request consumed its token
+            $this->runParallelBeforeQuery(
+                "/^DELETE FROM \\S*spbc_rate_limits/",
+                function () use (&$fresh) {
+                    spbc_2fa__step_token_issue($this->app_user);
+                    $fresh = get_user_meta($this->app_user->ID, 'spbc_2fa_step', true);
+                    return true;
+                }
+            );
+
+            $this->assertSame(array('auth', $this->app_user->ID), $this->postCode($login, $code));
+            $this->assertCount(1, $this->parallel_results, 'parallel login was not emulated');
+            $this->assertIsArray($fresh);
+            $this->assertSame($fresh, get_user_meta($this->app_user->ID, 'spbc_2fa_step', true));
+        });
+    }
+
+    public function testLoginByEmailWorksWithStepToken()
+    {
+        $this->inOneTimeStep(function ($code) {
+            $this->giveStepToken($this->app_user);
+
+            $result = $this->postCode($this->app_user->user_email, $code);
+
+            $this->assertSame(array('auth', $this->app_user->ID), $result);
+        });
+    }
+
+    public function testWrongCodeCountsFail()
+    {
+        $this->giveStepToken($this->app_user);
+
+        $result = $this->postCode($this->app_user->user_login, $this->wrongCode());
+
+        $this->assertCodeError($result, 1, $this->app_user);
+        $step = get_user_meta($this->app_user->ID, 'spbc_2fa_step', true);
+        $this->assertSame(1, $step['fails']);
+    }
+
+    public function testReusedCodeIsRejectedWithSeparateError()
+    {
+        $this->inOneTimeStep(function ($code) {
+            $this->giveStepToken($this->app_user);
+            $this->assertSame(array('auth', $this->app_user->ID), $this->postCode($this->app_user->user_login, $code));
+
+            $this->giveStepToken($this->app_user);
+            $this->assertCodeError($this->postCode($this->app_user->user_login, $code), 2, $this->app_user);
+        });
+    }
+
+    /**
+     * Two simultaneous submissions of the same token and code: only one may log in.
+     */
+    public function testConcurrentSubmissionOfSameCodeLogsInOnce()
+    {
+        $this->inOneTimeStep(function ($code) {
+            $this->giveStepToken($this->app_user);
+            $login = $this->app_user->user_login;
+
+            // The parallel request runs when the first one is about to consume the token
+            $this->runParallelBeforeQuery(
+                "/^DELETE FROM \\S*usermeta .*'spbc_2fa_step'/",
+                function () use ($login, $code) {
+                    return $this->postCode($login, $code);
+                }
+            );
+            $first = $this->postCode($login, $code);
+
+            $this->assertCount(1, $this->parallel_results, 'parallel request was not emulated');
+            $results = array($first, $this->parallel_results[0]);
+            $this->assertCount(
+                1,
+                array_filter($results, function ($result) {
+                    return $result[0] === 'auth';
+                }),
+                'Exactly one of concurrent requests must log in: ' . var_export($results, true)
+            );
+        });
+    }
+
+    /**
+     * Simultaneous wrong codes with the same token: each attempt is counted, the loser has to restart.
+     */
+    public function testConcurrentWrongCodesAreAllCounted()
+    {
+        $this->giveStepToken($this->app_user);
+        $login = $this->app_user->user_login;
+        $wrong = $this->wrongCode();
+
+        // The parallel request runs when the first one is about to reserve the attempt
+        $this->runParallelBeforeQuery(
+            "/^UPDATE \\S*usermeta SET meta_value .*'spbc_2fa_step'/",
+            function () use ($login, $wrong) {
+                return $this->postCode($login, $wrong);
+            }
+        );
+        $first = $this->postCode($login, $wrong);
+
+        $this->assertCount(1, $this->parallel_results, 'parallel request was not emulated');
+        $this->assertCodeError($this->parallel_results[0], 1, $this->app_user);
+        $this->assertRestart($first);
+
+        $step = get_user_meta($this->app_user->ID, 'spbc_2fa_step', true);
+        $this->assertSame(1, $step['fails']);
+    }
+
+    public function testUserOutsideOf2faRolesCannotLogInByCode()
+    {
+        global $spbc;
+
+        $spbc->settings['2fa__roles'] = array('administrator');
+        $this->giveStepToken($this->app_user);
+
+        $this->assertRestart($this->postCode($this->app_user->user_login, $this->currentCode()));
+    }
+
+    public function testEmailCodeRequiresStepToken()
+    {
+        global $spbc;
+
+        $login = $this->email_user->user_login;
+        $spbc->data['2fa_keys'][$login] = array('generated' => time(), 'val' => '123456');
+
+        $this->assertRestart($this->postCode($login, '123456'));
+        $this->assertArrayHasKey($login, $spbc->data['2fa_keys']);
+
+        $this->giveStepToken($this->email_user);
+        $this->assertSame(array('auth', $this->email_user->ID), $this->postCode($login, '123456'));
+        $this->assertArrayNotHasKey($login, $spbc->data['2fa_keys'], 'e-mail code is one-time');
+    }
+
+    public function testNonStringInputIsHandledAsWrongCode()
+    {
+        $this->giveStepToken($this->app_user);
+
+        $result = $this->postCode($this->app_user->user_login, array('000000'));
+        $this->assertCodeError($result, 1, $this->app_user);
+
+        $this->assertSame(array('none', null), $this->postCode(array('x'), '000000'));
+    }
+
+    public function testNonNumericCodeIsRejected()
+    {
+        $this->giveStepToken($this->app_user);
+
+        $this->assertCodeError($this->postCode($this->app_user->user_login, ' 12a456'), 1, $this->app_user);
+    }
+
+    public function testUnknownUserIsIgnored()
+    {
+        $this->assertSame(array('none', null), $this->postCode('spbc_2fa_nobody', '000000'));
+    }
+
+    public function testStepTokenIsDestroyedAfterMaxFails()
+    {
+        $this->giveStepToken($this->app_user);
+        $wrong = $this->wrongCode();
+
+        for ( $i = 1; $i < SPBC_2FA_MAX_ATTEMPTS; $i++ ) {
+            $this->assertCodeError($this->postCode($this->app_user->user_login, $wrong), 1, $this->app_user);
+        }
+        $this->assertRestart($this->postCode($this->app_user->user_login, $wrong));
+        $this->assertSame('', get_user_meta($this->app_user->ID, 'spbc_2fa_step', true));
+
+        // Even the correct code needs a new password step now
+        $this->assertRestart($this->postCode($this->app_user->user_login, $this->currentCode()));
+    }
+
+    public function testPerUserLimitSurvivesNewPasswordLoginsAndIpChange()
+    {
+        $wrong = $this->wrongCode();
+
+        for ( $i = 1; $i <= SPBC_2FA_USER_MAX_ATTEMPTS; $i++ ) {
+            // New password step and new IP on each attempt
+            $_SERVER['REMOTE_ADDR'] = '10.0.0.' . $i;
+            $this->resetRequestCache();
+            $this->giveStepToken($this->app_user);
+            $this->assertCodeError($this->postCode($this->app_user->user_login, $wrong), 1, $this->app_user);
+        }
+
+        $this->giveStepToken($this->app_user);
+        $this->assertSame(array('die', 403), $this->postCode($this->app_user->user_login, $this->currentCode()));
+        $this->assertSame('', get_user_meta($this->app_user->ID, 'spbc_2fa_step', true));
+
+        $this->giveStepToken($this->app_user);
+        $this->assertSame(array('die', 403), $this->postCode($this->app_user->user_login, $this->currentCode()));
+
+        // Other users are not affected
+        $this->giveStepToken($this->email_user);
+        $this->assertCodeError($this->postCode($this->email_user->user_login, '000000'), 1, $this->email_user);
+    }
+
+    /**
+     * A challenge issued for the old password (step cookie + e-mail code) must not complete the login after a reset.
+     */
+    public function testPasswordResetRevokesPendingChallenge()
+    {
+        global $spbc;
+
+        $login = $this->email_user->user_login;
+        $this->giveStepToken($this->email_user);
+        $spbc->data['2fa_keys'][$login] = array('generated' => time(), 'val' => '123456');
+
+        spbc_2fa__challenge_revoke($this->email_user);
+
+        $this->assertSame('', get_user_meta($this->email_user->ID, 'spbc_2fa_step', true));
+        $this->assertArrayNotHasKey($login, $spbc->data['2fa_keys']);
+        $this->assertRestart($this->postCode($login, '123456'));
+    }
+
+    public function testPerUserLimitIsResetByPasswordReset()
+    {
+        $this->inOneTimeStep(function ($code) {
+            $wrong = $this->wrongCode();
+
+            for ( $i = 0; $i <= SPBC_2FA_USER_MAX_ATTEMPTS; $i++ ) {
+                $this->giveStepToken($this->app_user);
+                $this->postCode($this->app_user->user_login, $wrong);
+            }
+            $this->giveStepToken($this->app_user);
+            $this->assertSame(array('die', 403), $this->postCode($this->app_user->user_login, $wrong));
+
+            spbc_2fa__user_rate_limiter_reset($this->app_user);
+
+            $this->giveStepToken($this->app_user);
+            $this->assertSame(array('auth', $this->app_user->ID), $this->postCode($this->app_user->user_login, $code));
+        });
+    }
+
+    public function testSuccessfulLoginResetsPerUserCounter()
+    {
+        $this->inOneTimeStep(function ($code) {
+            $wrong = $this->wrongCode();
+
+            for ( $i = 1; $i < SPBC_2FA_USER_MAX_ATTEMPTS; $i++ ) {
+                $this->giveStepToken($this->app_user);
+                $this->postCode($this->app_user->user_login, $wrong);
+            }
+            $this->giveStepToken($this->app_user);
+            $this->assertSame(array('auth', $this->app_user->ID), $this->postCode($this->app_user->user_login, $code));
+
+            for ( $i = 1; $i <= SPBC_2FA_USER_MAX_ATTEMPTS; $i++ ) {
+                $this->giveStepToken($this->app_user);
+                $this->assertCodeError($this->postCode($this->app_user->user_login, $wrong), 1, $this->app_user);
+            }
+        });
+    }
+
+    public function testDisabled2faIgnoresCode()
+    {
+        global $spbc;
+
+        $spbc->settings['2fa__enable'] = 0;
+        $this->giveStepToken($this->app_user);
+
+        $this->assertSame(array('none', null), $this->postCode($this->app_user->user_login, $this->currentCode()));
+    }
+
+    // ---------------------------------------------------------------- first factor
+
+    public function testPasswordStepIssuesTokenFor2faUser()
+    {
+        $result = $this->runInterruptible('spbc_authenticate', $this->app_user, $this->app_user->user_login);
+
+        $this->assertSame('redirect', $result[0]);
+        $this->assertStringContainsString('spbc_2fa_user=' . rawurlencode($this->app_user->user_login), $result[1]);
+
+        $step = get_user_meta($this->app_user->ID, 'spbc_2fa_step', true);
+        $this->assertIsArray($step);
+        $this->assertSame(0, $step['fails']);
+    }
+
+    public function testPasswordStepDoesNotIssueTokenForUserOutsideOf2faRoles()
+    {
+        $result = $this->runInterruptible('spbc_authenticate', $this->subscriber, $this->subscriber->user_login);
+
+        $this->assertSame(array('none', null), $result);
+        $this->assertSame('', get_user_meta($this->subscriber->ID, 'spbc_2fa_step', true));
+    }
+
+    // ---------------------------------------------------------------- code form
+
+    public function testCodeFormIsNotShownWithoutStepToken()
+    {
+        $_GET['spbc_2fa_user'] = $this->app_user->user_login;
+
+        list($result, $output) = $this->captureOutput('spbc_2fa__show_field');
+
+        $this->assertSame(array('none', null), $result);
+        $this->assertSame('', $output);
+    }
+
+    public function testCodeFormIsShownWithStepToken()
+    {
+        $_GET['spbc_2fa_user'] = $this->app_user->user_login;
+        $this->giveStepToken($this->app_user);
+
+        list(, $output) = $this->captureOutput('spbc_2fa__show_field');
+
+        $this->assertStringContainsString('spbc_2fa', $output);
+        $this->assertStringNotContainsString('CODE ALREADY USED', $output);
+    }
+
+    public function testReusedCodeMessageIsRenderedWithoutInnerHtml()
+    {
+        $_GET['spbc_2fa_user']  = $this->app_user->user_login;
+        $_GET['spbc_2fa_error'] = '2';
+        $this->giveStepToken($this->app_user);
+
+        list(, $output) = $this->captureOutput('spbc_2fa__show_field');
+
+        $this->assertStringContainsString('CODE ALREADY USED', $output);
+        $this->assertStringNotContainsString('{{', $output);
+
+        $error_script = substr($output, 0, strpos($output, '</script>'));
+        $this->assertStringContainsString('textContent', $error_script);
+        $this->assertStringNotContainsString('innerHTML', $error_script);
+    }
+
+    public function testReusedCodeMessageIsNotShownForEmailCode()
+    {
+        $_GET['spbc_2fa_user']  = $this->email_user->user_login;
+        $_GET['spbc_2fa_error'] = '2';
+        $this->giveStepToken($this->email_user);
+
+        list(, $output) = $this->captureOutput('spbc_2fa__show_field');
+
+        $this->assertStringNotContainsString('CODE ALREADY USED', $output);
+    }
+
+    // ---------------------------------------------------------------- 2FA application setup / removal
+
+    public function testReplaceMetaRemovesSecretEvenWithoutType()
+    {
+        delete_user_meta($this->app_user->ID, 'spbc_2fa_type');
+        update_user_meta($this->app_user->ID, 'spbc_2fa_last_step', 123);
+
+        $this->assertTrue(spbc_2fa__2fa_app_replace_meta($this->app_user));
+        $this->assertSame('', get_user_meta($this->app_user->ID, 'spbc_2fa_app_token', true));
+        $this->assertSame('', get_user_meta($this->app_user->ID, 'spbc_2fa_last_step', true));
+    }
+
+    public function testQrEndpointDoesNotExposeActivatedSecret()
+    {
+        $this->asAjaxUser($this->app_user);
+
+        list($result, $output) = $this->captureOutput('spbc_2fa__Get2FAAppQrCode');
+
+        $this->assertSame('die', $result[0]);
+        $response = json_decode($output, true);
+        $this->assertFalse($response['success']);
+        $this->assertStringNotContainsString(self::SECRET, $output);
+        $this->assertSame(self::SECRET, get_user_meta($this->app_user->ID, 'spbc_2fa_app_token', true));
+    }
+
+    public function testQrEndpointIssuesFreshSecretOnEachSetup()
+    {
+        delete_user_meta($this->email_user->ID, 'spbc_2fa_type');
+        $this->asAjaxUser($this->email_user);
+
+        $secrets = array();
+        for ( $i = 0; $i < 2; $i++ ) {
+            list(, $output) = $this->captureOutput('spbc_2fa__Get2FAAppQrCode');
+            $response = json_decode($output, true);
+
+            $this->assertTrue($response['success']);
+            $this->assertRegExp('/^[A-Z2-7]{32}$/', $response['data']['code']);
+            $this->assertSame($response['data']['code'], get_user_meta($this->email_user->ID, 'spbc_2fa_app_token', true));
+            $secrets[] = $response['data']['code'];
+        }
+
+        $this->assertNotSame($secrets[0], $secrets[1]);
+    }
+
+    public function testQrEndpointRequiresNonce()
+    {
+        $this->asAjaxUser($this->email_user);
+        $_REQUEST['security'] = 'wrong';
+
+        list($result) = $this->captureOutput('spbc_2fa__Get2FAAppQrCode');
+
+        $this->assertSame(array('die', 403), $result);
+        $this->assertSame('', get_user_meta($this->email_user->ID, 'spbc_2fa_app_token', true));
+    }
+
+    public function testActivationCodeMarksTimeStepAsUsed()
+    {
+        $this->inOneTimeStep(function ($code) {
+            delete_user_meta($this->app_user->ID, 'spbc_2fa_type');
+            $this->asAjaxUser($this->app_user);
+            $_POST['code'] = $code;
+
+            list(, $output) = $this->captureOutput('spbc_2fa__Check2FAAppCode');
+
+            $this->assertTrue(json_decode($output, true)['success']);
+            $this->assertSame('2fa_application', get_user_meta($this->app_user->ID, 'spbc_2fa_type', true));
+            $this->assertSame(
+                $this->timeStep(),
+                (int) get_user_meta($this->app_user->ID, 'spbc_2fa_last_step', true)
+            );
+
+            // The code used for activation can't be used to log in
+            $this->giveStepToken($this->app_user);
+            $this->assertCodeError($this->postCode($this->app_user->user_login, $code), 2, $this->app_user);
+        });
+    }
+
+    public function testEmailCodeIsSixDigits()
+    {
+        global $spbc;
+
+        for ( $i = 0; $i < 20; $i++ ) {
+            spbc_2fa__key_generate_and_store($this->email_user);
+            $this->assertRegExp(
+                '/^[1-9]\d{5}$/',
+                (string) $spbc->data['2fa_keys'][$this->email_user->user_login]['val']
+            );
+        }
+    }
+}
diff --git a/tests/lib/CleantalkSP/SpbctWP/G2FA/TestGoogleAuthenticator.php b/tests/lib/CleantalkSP/SpbctWP/G2FA/TestGoogleAuthenticator.php
new file mode 100644
index 000000000..0c031a28f
--- /dev/null
+++ b/tests/lib/CleantalkSP/SpbctWP/G2FA/TestGoogleAuthenticator.php
@@ -0,0 +1,38 @@
+<?php
+
+use CleantalkSP\SpbctWP\G2FA\GoogleAuthenticator;
+use PHPUnit\Framework\TestCase;
+
+class TestGoogleAuthenticator extends TestCase
+{
+    /**
+     * RFC 6238 test vector (SHA1, secret "12345678901234567890", T = 59 s), truncated to 6 digits.
+     */
+    public function testRfc6238Vector()
+    {
+        $ga = new GoogleAuthenticator();
+
+        $this->assertSame('287082', $ga->getCode('GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ', 1));
+    }
+
+    public function testCodeIsSixDigits()
+    {
+        $ga = new GoogleAuthenticator();
+
+        $this->assertRegExp('/^\d{6}$/', $ga->getCode($ga->generateSecret()));
+    }
+
+    public function testSecretIs160BitBase32()
+    {
+        $ga = new GoogleAuthenticator();
+
+        $secrets = array();
+        for ( $i = 0; $i < 50; $i++ ) {
+            $secret = $ga->generateSecret();
+            $this->assertRegExp('/^[A-Z2-7]{32}$/', $secret);
+            $secrets[] = $secret;
+        }
+
+        $this->assertCount(50, array_unique($secrets));
+    }
+}
diff --git a/tests/lib/CleantalkSP/SpbctWP/RateLimiter/TestSpbcUserRateLimiter.php b/tests/lib/CleantalkSP/SpbctWP/RateLimiter/TestSpbcUserRateLimiter.php
new file mode 100644
index 000000000..b3cd41007
--- /dev/null
+++ b/tests/lib/CleantalkSP/SpbctWP/RateLimiter/TestSpbcUserRateLimiter.php
@@ -0,0 +1,151 @@
+<?php
+
+use CleantalkSP\Common\RateLimit\RateLimiterConfig;
+use CleantalkSP\SpbctWP\DB\TablesCreator;
+use CleantalkSP\SpbctWP\SpbcRateLimit\SpbcUserRateLimiter;
+use CleantalkSP\Variables\Server;
+
+class TestSpbcUserRateLimiter extends \SpbcTestCase
+{
+    const TYPE = 'test_user_limiter';
+
+    private $saved_server;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $tables_creator = new TablesCreator();
+        $tables_creator->createTable(SPBC_TBL_RATE_LIMITS);
+        $this->deleteRows();
+
+        $this->saved_server = $_SERVER;
+        $this->setRequest('10.0.0.1', 'PHPUnit');
+    }
+
+    protected function tearDown(): void
+    {
+        $this->deleteRows();
+        $_SERVER = $this->saved_server;
+        $this->resetRequestCache();
+
+        parent::tearDown();
+    }
+
+    private function deleteRows()
+    {
+        global $wpdb;
+        $wpdb->query($wpdb->prepare('DELETE FROM ' . SPBC_TBL_RATE_LIMITS . ' WHERE type = %s', self::TYPE));
+    }
+
+    private function resetRequestCache()
+    {
+        Server::getInstance()->variables = array();
+        \CleantalkSP\SpbctWP\Helpers\IP::getInstance()->ips_stored = array();
+    }
+
+    private function setRequest($ip, $ua)
+    {
+        $_SERVER['REMOTE_ADDR']     = $ip;
+        $_SERVER['HTTP_USER_AGENT'] = $ua;
+        $this->resetRequestCache();
+    }
+
+    private function limiter($user_id, $limit = 2)
+    {
+        return new SpbcUserRateLimiter(new RateLimiterConfig(self::TYPE, $limit, 60), $user_id);
+    }
+
+    public function testLocksAfterLimit()
+    {
+        $this->assertTrue($this->limiter(1)->checkPassed());
+        $this->assertTrue($this->limiter(1)->checkPassed());
+
+        $limiter = $this->limiter(1);
+        $this->assertFalse($limiter->checkPassed());
+        $this->assertTrue($limiter->process_ok);
+    }
+
+    public function testCounterIsPerUser()
+    {
+        for ( $i = 0; $i < 3; $i++ ) {
+            $this->limiter(1)->checkPassed();
+        }
+
+        $this->assertFalse($this->limiter(1)->checkPassed());
+        $this->assertTrue($this->limiter(2)->checkPassed());
+    }
+
+    public function testIpChangeDoesNotResetCounter()
+    {
+        for ( $i = 1; $i <= 2; $i++ ) {
+            $this->setRequest('10.0.0.' . $i, 'PHPUnit');
+            $this->assertTrue($this->limiter(1)->checkPassed());
+        }
+
+        $this->setRequest('10.0.0.3', 'PHPUnit');
+        $this->assertFalse($this->limiter(1)->checkPassed());
+    }
+
+    public function testResetUnlocksOnlyGivenUser()
+    {
+        for ( $i = 0; $i < 3; $i++ ) {
+            $this->limiter(1)->checkPassed();
+            $this->limiter(2)->checkPassed();
+        }
+
+        $this->limiter(1)->reset();
+
+        $this->assertTrue($this->limiter(1)->checkPassed());
+        $this->assertFalse($this->limiter(2)->checkPassed());
+    }
+
+    public function hostileUserAgentProvider()
+    {
+        return array(
+            'long ascii'                     => array(str_repeat('A', 1000)),
+            'multibyte char split at 200'    => array(str_repeat('A', 199) . "\xC3\xA9"),
+            'invalid utf-8 byte'             => array('Mozilla' . "\xFF\xFE"),
+            '4-byte utf-8 char'              => array('Mozilla ' . "\xF0\x9F\x98\x80"),
+            'long multibyte'                 => array(str_repeat("\xD0\x96", 500)),
+        );
+    }
+
+    /**
+     * An attacker-controlled UA must not fail the insert: the limiter fails open on DB errors.
+     *
+     * @dataProvider hostileUserAgentProvider
+     */
+    public function testHostileUserAgentDoesNotBreakCounting($ua)
+    {
+        $this->assertLockedAfterOneAttempt($ua);
+    }
+
+    /**
+     * @dataProvider hostileUserAgentProvider
+     */
+    public function testHostileUserAgentDoesNotBreakCountingInStrictMode($ua)
+    {
+        global $wpdb;
+
+        $sql_mode = $wpdb->get_var('SELECT @@SESSION.sql_mode');
+        $wpdb->query("SET SESSION sql_mode = 'STRICT_ALL_TABLES'");
+
+        try {
+            $this->assertLockedAfterOneAttempt($ua);
+        } finally {
+            $wpdb->query($wpdb->prepare('SET SESSION sql_mode = %s', $sql_mode));
+        }
+    }
+
+    private function assertLockedAfterOneAttempt($ua)
+    {
+        $this->setRequest('10.0.0.1', $ua);
+
+        $this->assertTrue($this->limiter(1, 1)->checkPassed());
+
+        $limiter = $this->limiter(1, 1);
+        $this->assertFalse($limiter->checkPassed(), 'limiter must not fail open on a hostile UA');
+        $this->assertTrue($limiter->process_ok);
+    }
+}

From 1fc6c63b7a01ae9c703ffed2c7cc7ed30f5f08ff Mon Sep 17 00:00:00 2001
From: AntonV1211 <39616556+AntonV1211@users.noreply.github.com>
Date: Tue, 27 Oct 2026 11:45:52 +0700
Subject: [PATCH 16/17] Fix. FSW. Restriction of access to FSW, edits for wpms,
 restriction for file actions (#762)

* Dev. Git. Added the Skills folder to .gitignore

* Fix. FSW. Restriction of access to FSW, edits for wpms, restriction for file actions

* Fix. FSW. Limitation for actions of oscron and delete-trigger

* Dev. Code. Review corrections

* Dev. Activator. Switching to the wp_initialize_site hook when adding a new blog
---
 inc/spbc-admin.php                            |   2 +-
 lib/CleantalkSP/SpbctWP/Activator.php         |   7 +-
 .../SpbctWP/FSWatcher/Analyzer/Analyzer.php   |  96 ++++-----
 .../SpbctWP/FSWatcher/Controller.php          |   6 +-
 lib/CleantalkSP/SpbctWP/ListTable.php         |  20 ++
 lib/CleantalkSP/SpbctWP/Scanner/Cure.php      |   4 +-
 lib/CleantalkSP/SpbctWP/Scanner/Helper.php    |  11 ++
 .../Scanner/ScannerActions/BackupsActions.php |   2 +-
 .../SpbctWP/Scanner/ScannerAjaxEndpoints.php  |  16 +-
 .../SpbctWP/Settings/SettingsGeneralReact.php |   5 +-
 lib/CleantalkSP/SpbctWP/Views/Settings.php    |   3 +-
 security-malware-firewall.php                 |   6 +-
 .../SpbctWP/ActivatorNewBlogTest.php          |  52 +++++
 .../FSWatcher/AnalyzerViewFileTest.php        | 185 ++++++++++++++++++
 14 files changed, 349 insertions(+), 66 deletions(-)
 create mode 100644 tests/lib/CleantalkSP/SpbctWP/ActivatorNewBlogTest.php
 create mode 100644 tests/lib/CleantalkSP/SpbctWP/FSWatcher/AnalyzerViewFileTest.php

diff --git a/inc/spbc-admin.php b/inc/spbc-admin.php
index 365d6edd2..ae2bd050d 100644
--- a/inc/spbc-admin.php
+++ b/inc/spbc-admin.php
@@ -840,7 +840,7 @@ function spbc_get_authorized_admins($direct_call = false)
 
 function spbc_action_shuffle_salts()
 {
-    spbc_check_ajax_referer('spbc_secret_nonce', 'security');
+    spbc_check_ajax_referer('spbc_secret_nonce', 'security', true, true, true);
 
     global $spbc;
     $salts_array = array(
diff --git a/lib/CleantalkSP/SpbctWP/Activator.php b/lib/CleantalkSP/SpbctWP/Activator.php
index e4d059bb4..25670ac56 100644
--- a/lib/CleantalkSP/SpbctWP/Activator.php
+++ b/lib/CleantalkSP/SpbctWP/Activator.php
@@ -22,7 +22,7 @@ public static function activation($network, $redirect = true)
 
         CDNHeadersChecker::sendCDNCheckerRequest();
 
-        if ( current_action() !== 'wp_insert_site' ) {
+        if ( current_action() !== 'wp_initialize_site' ) {
             delete_option('spbc_deactivation_in_process');
         }
 
@@ -178,7 +178,8 @@ public static function addActionForNetworkBlogLegacy($wp_version)
         if ( version_compare($wp_version, '5.1.0', '<') ) {
             add_action('wpmu_new_blog', array(static::class, 'actionNewBlogLegacyMode'), 10, 6);
         } else {
-            add_action('wp_insert_site', array(static::class, 'actionNewBlog'), 10, 1);
+            // Core creates the blog tables on wp_initialize_site with priority 10, so run after it
+            add_action('wp_initialize_site', array(static::class, 'actionNewBlog'), 11, 1);
         }
     }
 
@@ -215,7 +216,7 @@ public static function actionNewBlogLegacyMode($blog_id, $_user_id, $_domain, $_
      * Wrapper for spbc_activation__new_blog()
      *
      * @param $new_site
-     * @psalm-suppress UnusedMethod - this method called via hook wp_insert_site/wpmu_new_blog
+     * @psalm-suppress UnusedMethod - this method called via hook wp_initialize_site/wpmu_new_blog
      */
     public static function actionNewBlog($new_site)
     {
diff --git a/lib/CleantalkSP/SpbctWP/FSWatcher/Analyzer/Analyzer.php b/lib/CleantalkSP/SpbctWP/FSWatcher/Analyzer/Analyzer.php
index 530db2f71..5f3f66f86 100644
--- a/lib/CleantalkSP/SpbctWP/FSWatcher/Analyzer/Analyzer.php
+++ b/lib/CleantalkSP/SpbctWP/FSWatcher/Analyzer/Analyzer.php
@@ -4,7 +4,7 @@
 
 use CleantalkSP\SpbctWP\FSWatcher\Controller;
 use CleantalkSP\SpbctWP\FSWatcher\Logger;
-use CleantalkSP\SpbctWP\FSWatcher\Storage\FileStorage;
+use CleantalkSP\SpbctWP\Scanner\Helper;
 
 class Analyzer
 {
@@ -44,82 +44,88 @@ public static function getCompareResult()
      */
     public static function getViewFile()
     {
-        $path = isset($_POST['fswatcher_file_path']) ? $_POST['fswatcher_file_path'] : false;
+        $path = isset($_POST['fswatcher_file_path']) ? wp_unslash($_POST['fswatcher_file_path']) : false;
+        $first = isset($_POST['fswatcher__first_date']) ? filter_var($_POST['fswatcher__first_date'], FILTER_VALIDATE_INT) : false;
+        $second = isset($_POST['fswatcher__second_date']) ? filter_var($_POST['fswatcher__second_date'], FILTER_VALIDATE_INT) : false;
 
-        $journals_first = isset($_POST['fswatcher__first_date']) ? $_POST['fswatcher__first_date'] : false;
-        $journals_second = isset($_POST['fswatcher__second_date']) ? $_POST['fswatcher__second_date'] : false;
-        $journals = array($journals_first, $journals_second);
+        if (!$path || !is_string($path)) {
+            throw new \Exception('File path is incorrect.');
+        }
 
-        if (!$path || !is_file($path)) {
+        $real_path = realpath($path);
+        if ($real_path === false || !is_file($real_path)) {
             throw new \Exception('File path is incorrect.');
         }
 
-        if (!$journals[0] || !$journals[1]) {
+        if (self::isWpConfigFile($path, $real_path) && !Helper::canViewWpConfigContents()) {
+            throw new \Exception(Helper::getWpConfigViewRestrictedMessage());
+        }
+
+        if (!$first || !$second) {
             throw new \Exception('Provided journals paths are incorrect.');
         }
 
-        $path_found_in_journal = false;
+        $compare_result = self::getCompareResult();
 
-        foreach ($journals as $journal_id) {
-            if (
-                self::isFileOfFSWJournal($path, $journal_id) &&
-                self::isFileOfFSWJournalAfterCompareJournals($path)
-            ) {
-                $path_found_in_journal = true;
-                break;
-            }
+        if (!is_array($compare_result) || !self::isPathInCompareResult($real_path, $compare_result)) {
+            throw new \Exception('The file is out of FSWatcher journals.');
         }
 
-        if (!$path_found_in_journal) {
-            throw new \Exception('The file is out of FSWatcher journals.');
+        $contents = file_get_contents($real_path);
+        if ($contents === false) {
+            throw new \Exception('File path is incorrect.');
         }
 
-        return esc_html__(file_get_contents($path));
+        return esc_html__($contents);
     }
 
     /**
-     * @param $path
-     * @param $journal_id
+     * Whether the file is the WordPress wp-config.php (site root or one level above), symlinks resolved.
+     *
+     * @param string $path Path as requested
+     * @param string $real_path Resolved path
      * @return bool
      */
-    private static function isFileOfFSWJournal($path, $journal_id)
+    private static function isWpConfigFile($path, $real_path)
     {
-        $storage = new FileStorage();
-        $journal_parsed = $storage->getJournal($journal_id);
-        $analyzer = new Analyzer();
-        $journal_parsed = $analyzer->uncompress($journal_parsed, true);
-        if (
-                strpos($journal_parsed, $path) !== false ||
-                strpos($journal_parsed, stripslashes($path)) !== false
-        ) {
+        if (Helper::isWpConfigPath($path) || Helper::isWpConfigPath($real_path)) {
             return true;
         }
+
+        foreach (array(ABSPATH . 'wp-config.php', ABSPATH . '../wp-config.php') as $config_file) {
+            $real_config_file = realpath($config_file);
+            if ($real_config_file !== false && $real_config_file === $real_path) {
+                return true;
+            }
+        }
+
         return false;
     }
 
     /**
-     * @param $path
+     * Exact match of the resolved path against the paths of the journals diff.
+     *
+     * @param string $real_path
+     * @param array $compare_result Result of self::compare()
      * @return bool
      */
-    private static function isFileOfFSWJournalAfterCompareJournals($path)
+    private static function isPathInCompareResult($real_path, array $compare_result)
     {
-        $journal_result_compare = self::getCompareResult();
-        $journal_result_string = '';
-        foreach ($journal_result_compare as $journal_result) {
-            if (count($journal_result) > 0) {
-                foreach ($journal_result as $value) {
-                    $journal_result_string .= implode(' , ', $value);
+        foreach ($compare_result as $entries) {
+            if (!is_array($entries)) {
+                continue;
+            }
+            foreach ($entries as $entry) {
+                if (!isset($entry[0])) {
+                    continue;
+                }
+                $entry_real_path = realpath((string) $entry[0]);
+                if ($entry_real_path !== false && $entry_real_path === $real_path) {
+                    return true;
                 }
             }
         }
 
-        if (
-            strpos($journal_result_string, $path) !== false ||
-            strpos($journal_result_string, stripslashes($path)) !== false
-        ) {
-            return true;
-        }
-
         return false;
     }
 
diff --git a/lib/CleantalkSP/SpbctWP/FSWatcher/Controller.php b/lib/CleantalkSP/SpbctWP/FSWatcher/Controller.php
index b611a6143..85e16473c 100644
--- a/lib/CleantalkSP/SpbctWP/FSWatcher/Controller.php
+++ b/lib/CleantalkSP/SpbctWP/FSWatcher/Controller.php
@@ -122,7 +122,7 @@ private function checkRateLimit()
 
     public static function ajaxCompare()
     {
-        spbc_check_ajax_referer('spbc_secret_nonce', 'security');
+        spbc_check_ajax_referer('spbc_secret_nonce', 'security', true, true, true);
         if (self::$debug) {
             Logger::log('run compare file system logs');
         }
@@ -141,7 +141,7 @@ public static function ajaxCompare()
 
     public function ajaxViewFile()
     {
-        spbc_check_ajax_referer('spbc_secret_nonce', 'security');
+        spbc_check_ajax_referer('spbc_secret_nonce', 'security', true, true, true);
         if (self::$debug) {
             Logger::log('run view file method');
         }
@@ -165,7 +165,7 @@ public function ajaxViewFile()
      */
     public function ajaxCreateSnapshot()
     {
-        spbc_check_ajax_referer('spbc_secret_nonce', 'security');
+        spbc_check_ajax_referer('spbc_secret_nonce', 'security', true, true, true);
         if (self::$debug) {
             Logger::log('run scan file system');
         }
diff --git a/lib/CleantalkSP/SpbctWP/ListTable.php b/lib/CleantalkSP/SpbctWP/ListTable.php
index bec47674b..cc97bbbe6 100644
--- a/lib/CleantalkSP/SpbctWP/ListTable.php
+++ b/lib/CleantalkSP/SpbctWP/ListTable.php
@@ -1080,6 +1080,26 @@ public static function ajaxRowActionHandler()
         spbc_check_ajax_referer('spbc_secret_nonce', 'security');
 
         $action = Post::getString('add_action');
+
+        // Actions that change files, OS crontab or DB triggers shared by the whole network are for Super Admins only on a multisite
+        $network_wide_actions = array(
+            'delete',
+            'delete_quarantine',
+            'replace',
+            'quarantine',
+            'restore',
+            'enable_oscron_task',
+            'disable_oscron_task',
+            'delete-trigger',
+        );
+        if ( in_array($action, $network_wide_actions, true) && ! \CleantalkSP\SpbctWP\Scanner\Helper::canCurrentUserDoNetworkWideActions() ) {
+            wp_send_json(array(
+                'temp_html' => '<div class="spbc-popup-msg popup--red">'
+                    . esc_html__('Only a Super Admin can perform this action on a multisite network.', 'security-malware-firewall')
+                    . '</div>'
+            ));
+        }
+
         $is_frontend_malware_action = $action === 'disapprove_page' || $action === 'approve_page';
 
         if ( $action !== 'restore' && ! $is_frontend_malware_action &&
diff --git a/lib/CleantalkSP/SpbctWP/Scanner/Cure.php b/lib/CleantalkSP/SpbctWP/Scanner/Cure.php
index 590d7fbf5..384129296 100644
--- a/lib/CleantalkSP/SpbctWP/Scanner/Cure.php
+++ b/lib/CleantalkSP/SpbctWP/Scanner/Cure.php
@@ -337,7 +337,7 @@ private static function idsForBulkRequest($allowed_statuses)
      */
     public static function cureSelectedAction()
     {
-        spbc_check_ajax_referer('spbc_secret_nonce', 'security');
+        spbc_check_ajax_referer('spbc_secret_nonce', 'security', true, true, true);
 
         $ids = self::idsForBulkRequest(array('critical', 'cure_log'));
 
@@ -395,7 +395,7 @@ public static function cureSelectedAction()
 
     public static function restoreSelectedAction()
     {
-        spbc_check_ajax_referer('spbc_secret_nonce', 'security');
+        spbc_check_ajax_referer('spbc_secret_nonce', 'security', true, true, true);
 
         $ids = self::idsForBulkRequest(array('cure_log'));
 
diff --git a/lib/CleantalkSP/SpbctWP/Scanner/Helper.php b/lib/CleantalkSP/SpbctWP/Scanner/Helper.php
index bcb947c4e..86d0b5402 100644
--- a/lib/CleantalkSP/SpbctWP/Scanner/Helper.php
+++ b/lib/CleantalkSP/SpbctWP/Scanner/Helper.php
@@ -332,6 +332,17 @@ public static function canViewWpConfigContents()
         return function_exists('is_super_admin') && is_super_admin();
     }
 
+    /**
+     * Whether the current user may act on files shared by the whole network.
+     * On a multisite the file system is shared by all subsites, so only Super Admins may do it.
+     *
+     * @return bool
+     */
+    public static function canCurrentUserDoNetworkWideActions()
+    {
+        return ! is_multisite() || self::canViewWpConfigContents();
+    }
+
     /**
      * @return string
      */
diff --git a/lib/CleantalkSP/SpbctWP/Scanner/ScannerActions/BackupsActions.php b/lib/CleantalkSP/SpbctWP/Scanner/ScannerActions/BackupsActions.php
index ae57289f6..d293db2cc 100644
--- a/lib/CleantalkSP/SpbctWP/Scanner/ScannerActions/BackupsActions.php
+++ b/lib/CleantalkSP/SpbctWP/Scanner/ScannerActions/BackupsActions.php
@@ -15,7 +15,7 @@ class BackupsActions
      */
     protected static function checkAjaxReferer($action = 'spbc_secret_nonce', $query_arg = 'security')
     {
-        spbc_check_ajax_referer($action, $query_arg);
+        spbc_check_ajax_referer($action, $query_arg, true, true, true);
     }
 
     /**
diff --git a/lib/CleantalkSP/SpbctWP/Scanner/ScannerAjaxEndpoints.php b/lib/CleantalkSP/SpbctWP/Scanner/ScannerAjaxEndpoints.php
index 588296764..a0e96764a 100644
--- a/lib/CleantalkSP/SpbctWP/Scanner/ScannerAjaxEndpoints.php
+++ b/lib/CleantalkSP/SpbctWP/Scanner/ScannerAjaxEndpoints.php
@@ -33,7 +33,7 @@ public static function getLastScanInfo()
      */
     public static function restoreFileFromBackup()
     {
-        spbc_check_ajax_referer('spbc_secret_nonce', 'security');
+        spbc_check_ajax_referer('spbc_secret_nonce', 'security', true, true, true);
 
         $file_fast_hash = isset($_POST['file_fast_hash']) ? esc_sql($_POST['file_fast_hash']) : null;
 
@@ -52,7 +52,7 @@ public static function restoreFileFromBackup()
      */
     public static function cureFile()
     {
-        spbc_check_ajax_referer('spbc_secret_nonce', 'security');
+        spbc_check_ajax_referer('spbc_secret_nonce', 'security', true, true, true);
 
         $file_fast_hash = isset($_POST['file_fast_hash']) ? esc_sql($_POST['file_fast_hash']) : null;
 
@@ -139,7 +139,7 @@ public static function loadMoreScanLogs()
      */
     public static function deleteFile()
     {
-        spbc_check_ajax_referer('spbc_secret_nonce', 'security');
+        spbc_check_ajax_referer('spbc_secret_nonce', 'security', true, true, true);
 
         $time_start = microtime(true);
 
@@ -218,7 +218,7 @@ public static function deleteFileFromAnalysisLog($direct_call = false)
     public static function restoreFromQuarantine($direct_call = false, $file_id = null)
     {
         if ( ! $direct_call) {
-            spbc_check_ajax_referer('spbc_secret_nonce', 'security');
+            spbc_check_ajax_referer('spbc_secret_nonce', 'security', true, true, true);
         }
 
         $file_id = $direct_call ? $file_id : Post::get('file_id', 'hash');
@@ -277,7 +277,7 @@ public static function restoreFromQuarantine($direct_call = false, $file_id = nu
     public static function doQuarantineFile($direct_call = false, $file_id = null)
     {
         if ( ! $direct_call ) {
-            spbc_check_ajax_referer('spbc_secret_nonce', 'security');
+            spbc_check_ajax_referer('spbc_secret_nonce', 'security', true, true, true);
         }
 
         $file_id = $direct_call ? $file_id : Post::get('file_id', 'hash');
@@ -302,7 +302,7 @@ public static function doQuarantineFile($direct_call = false, $file_id = null)
     public static function deleteFromQuarantine($direct_call = false, $file_id = null)
     {
         if ( ! $direct_call ) {
-            spbc_check_ajax_referer('spbc_secret_nonce', 'security');
+            spbc_check_ajax_referer('spbc_secret_nonce', 'security', true, true, true);
         }
 
         $file_id = $direct_call ? $file_id : Post::get('file_id', 'hash');
@@ -327,7 +327,7 @@ public static function deleteFromQuarantine($direct_call = false, $file_id = nul
     public static function replaceFileWithOriginal($direct_call = false, $file_id = null)
     {
         if ( ! $direct_call) {
-            spbc_check_ajax_referer('spbc_secret_nonce', 'security');
+            spbc_check_ajax_referer('spbc_secret_nonce', 'security', true, true, true);
         }
 
         $file_id = $direct_call ? $file_id : Post::get('file_id', 'hash');
@@ -371,7 +371,7 @@ public static function viewPage()
      */
     public static function viewFile()
     {
-        spbc_check_ajax_referer('spbc_secret_nonce', 'security');
+        spbc_check_ajax_referer('spbc_secret_nonce', 'security', true, true, true);
 
         $output = FileSystemActions::viewFile(Post::get('file_id', 'hash'));
 
diff --git a/lib/CleantalkSP/SpbctWP/Settings/SettingsGeneralReact.php b/lib/CleantalkSP/SpbctWP/Settings/SettingsGeneralReact.php
index aed31a093..d83b310ab 100644
--- a/lib/CleantalkSP/SpbctWP/Settings/SettingsGeneralReact.php
+++ b/lib/CleantalkSP/SpbctWP/Settings/SettingsGeneralReact.php
@@ -233,7 +233,10 @@ private static function orderGroupElements($grouped_settings)
         $ordering_ruleset = self::getOrderingRuleset();
 
         // UI-only field (not a saved option); rendered by React ActionShuffleSalts
-        $grouped_settings['auth_and_logging_in']['action_shuffle_salts'] = true;
+        // It rewrites the wp-config.php shared by the whole network, so on a multisite it is for Super Admins only
+        if ( \CleantalkSP\SpbctWP\Scanner\Helper::canCurrentUserDoNetworkWideActions() ) {
+            $grouped_settings['auth_and_logging_in']['action_shuffle_salts'] = true;
+        }
 
         foreach ($ordering_ruleset as $group_name => $ruleset) {
             $ordered = [];
diff --git a/lib/CleantalkSP/SpbctWP/Views/Settings.php b/lib/CleantalkSP/SpbctWP/Views/Settings.php
index 0c091cc13..7622824a6 100644
--- a/lib/CleantalkSP/SpbctWP/Views/Settings.php
+++ b/lib/CleantalkSP/SpbctWP/Views/Settings.php
@@ -108,7 +108,8 @@ public static function page()
                 "displayDebug" => $display_debug,
                 "isWPMSMainSite" => is_main_site(),
                 "isSecFWEnabled" => (bool) $spbc->settings['secfw__enabled'],
-                "isFsWatcherEnabled" => (bool) $spbc->settings['scanner__fs_watcher'],
+                "isFsWatcherEnabled" => (bool) $spbc->settings['scanner__fs_watcher']
+                    && \CleantalkSP\SpbctWP\Scanner\Helper::canCurrentUserDoNetworkWideActions(),
                 "isVulnerabilityCheckEnabled" => (bool) $spbc->settings['vulnerability_check__enable_cron'],
             ];
         }
diff --git a/security-malware-firewall.php b/security-malware-firewall.php
index 8f66b2b3a..1f0da6179 100644
--- a/security-malware-firewall.php
+++ b/security-malware-firewall.php
@@ -1437,9 +1437,10 @@ function spbc_PHP_logs__send()
  * @param string|false $query_arg query arg where to search a nonce
  * @param bool $die is need to stop the flow with 403 wp_die()
  * @param bool $strict_admins is need to check current user capabilities comply to admin's
+ * @param bool $network_wide on a multisite also require a Super Admin (for actions on the file system or wp-config.php shared by the whole network)
  * @return bool
  */
-function spbc_check_ajax_referer($action = -1, $query_arg = false, $die = true, $strict_admins = true)
+function spbc_check_ajax_referer($action = -1, $query_arg = false, $die = true, $strict_admins = true, $network_wide = false)
 {
     $result = true;
     if (function_exists('check_ajax_referer')) {
@@ -1453,6 +1454,9 @@ function spbc_check_ajax_referer($action = -1, $query_arg = false, $die = true,
     if ($result && $strict_admins) {
         $result = current_user_can('manage_options');
     }
+    if ($result && $network_wide) {
+        $result = \CleantalkSP\SpbctWP\Scanner\Helper::canCurrentUserDoNetworkWideActions();
+    }
     if (!$result && $die) {
         wp_die('-1', 403);
     }
diff --git a/tests/lib/CleantalkSP/SpbctWP/ActivatorNewBlogTest.php b/tests/lib/CleantalkSP/SpbctWP/ActivatorNewBlogTest.php
new file mode 100644
index 000000000..f0eb0dccd
--- /dev/null
+++ b/tests/lib/CleantalkSP/SpbctWP/ActivatorNewBlogTest.php
@@ -0,0 +1,52 @@
+<?php
+
+use CleantalkSP\SpbctWP\Activator;
+
+class ActivatorNewBlogTest extends SpbcTestCase
+{
+    /**
+     * New blog must be initialized after the core created its tables.
+     * @test
+     */
+    public function testNewBlogHookRunsAfterCoreTablesCreation()
+    {
+        $this->assertFalse(has_action('wp_insert_site', array(Activator::class, 'actionNewBlog')));
+        $this->assertSame(11, has_action('wp_initialize_site', array(Activator::class, 'actionNewBlog')));
+    }
+
+    /**
+     * @test
+     */
+    public function testNewBlogGetsPluginDataOnNetworkActivePlugin()
+    {
+        if ( ! is_multisite() ) {
+            $this->markTestSkipped('Multisite only, run with WP_MULTISITE=1.');
+        }
+
+        // Core creates the blog tables here, the plugin must run later
+        $this->assertSame(10, has_action('wp_initialize_site', 'wp_initialize_site'));
+
+        $plugin = 'security-malware-firewall/security-malware-firewall.php';
+        $active_plugins_backup = get_site_option('active_sitewide_plugins');
+        $active_plugins = is_array($active_plugins_backup) ? $active_plugins_backup : array();
+        $active_plugins[$plugin] = time();
+        update_site_option('active_sitewide_plugins', $active_plugins);
+
+        $blog_id = wpmu_create_blog(
+            (string) get_network()->domain,
+            '/spbc-new-blog-' . uniqid() . '/',
+            'SPBC new blog test',
+            1
+        );
+
+        try {
+            $this->assertIsInt($blog_id);
+            $this->assertNotFalse(get_blog_option($blog_id, 'spbc_data'));
+        } finally {
+            update_site_option('active_sitewide_plugins', $active_plugins_backup);
+            if (is_int($blog_id)) {
+                wp_delete_site($blog_id);
+            }
+        }
+    }
+}
diff --git a/tests/lib/CleantalkSP/SpbctWP/FSWatcher/AnalyzerViewFileTest.php b/tests/lib/CleantalkSP/SpbctWP/FSWatcher/AnalyzerViewFileTest.php
new file mode 100644
index 000000000..81c7abe32
--- /dev/null
+++ b/tests/lib/CleantalkSP/SpbctWP/FSWatcher/AnalyzerViewFileTest.php
@@ -0,0 +1,185 @@
+<?php
+
+use CleantalkSP\SpbctWP\FSWatcher\Analyzer\Analyzer;
+use CleantalkSP\SpbctWP\FSWatcher\Controller;
+use CleantalkSP\SpbctWP\FSWatcher\Storage\FileStorage;
+
+class AnalyzerViewFileTest extends SpbcTestCase
+{
+    const FIRST_JOURNAL = 1900000001;
+    const SECOND_JOURNAL = 1900000002;
+
+    /**
+     * @var string[]
+     */
+    private $files_to_remove = array();
+
+    /**
+     * @var mixed
+     */
+    private $storage_backup;
+
+    /**
+     * @var int
+     */
+    private $user_backup;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->storage_backup = Controller::$storage;
+        $this->user_backup = get_current_user_id();
+        Controller::$storage = FileStorage::class;
+        wp_set_current_user(1);
+    }
+
+    protected function tearDown(): void
+    {
+        foreach ($this->files_to_remove as $file) {
+            if (is_file($file)) {
+                unlink($file);
+            }
+        }
+        $this->files_to_remove = array();
+        Controller::$storage = $this->storage_backup;
+        wp_set_current_user($this->user_backup);
+        $_POST = array();
+        parent::tearDown();
+    }
+
+    /**
+     * Site-root wp-config.php must not be readable through the viewer by a non Super Admin.
+     * @test
+     */
+    public function testWpConfigInDiffIsDeniedWithoutSuperAdmin()
+    {
+        $wp_config = $this->prepareWpConfig();
+        $this->writeJournals(array($wp_config => '1000'), array($wp_config => '2000'));
+        wp_set_current_user(0);
+
+        $result = $this->view($wp_config);
+
+        $this->assertArrayHasKey('error', $result, 'wp-config.php contents must not be returned');
+        $this->assertStringContainsString('Super Admin', $result['error']);
+    }
+
+    /**
+     * The same file stays viewable for a Super Admin.
+     * @test
+     */
+    public function testWpConfigInDiffIsReadableForSuperAdmin()
+    {
+        $wp_config = $this->prepareWpConfig();
+        $this->writeJournals(array($wp_config => '1000'), array($wp_config => '2000'));
+        wp_set_current_user(1);
+
+        $result = $this->view($wp_config);
+
+        $this->assertArrayHasKey('data', $result);
+        $this->assertStringContainsString('spbc-fsw-test-marker', $result['data']);
+    }
+
+    /**
+     * Legit case: ordinary changed file is viewable.
+     * @test
+     */
+    public function testChangedFileIsReadable()
+    {
+        $file = $this->makeTempFile('ordinary');
+        $this->writeJournals(array($file => '1000'), array($file => '2000'));
+
+        $result = $this->view($file);
+
+        $this->assertArrayHasKey('data', $result);
+        $this->assertStringContainsString('spbc-fsw-test-marker', $result['data']);
+    }
+
+    /**
+     * A file that exists but is not part of the diff must be rejected.
+     * @test
+     */
+    public function testFileOutOfDiffIsDenied()
+    {
+        $file = $this->makeTempFile('victim');
+        $other = $this->makeTempFile('other');
+        $this->writeJournals(array($other => '1000'), array($other => '2000'));
+
+        $result = $this->view($file);
+
+        $this->assertArrayHasKey('error', $result);
+        $this->assertStringContainsString('out of FSWatcher journals', $result['error']);
+    }
+
+    /**
+     * Path that is only a substring of a journal line must not pass.
+     * @test
+     */
+    public function testPathThatIsOnlySubstringOfJournalEntryIsDenied()
+    {
+        $file = $this->makeTempFile('victim');
+        $longer = $file . '.bak';
+        $this->writeJournals(array($longer => '1000'), array($longer => '2000'));
+
+        $result = $this->view($file);
+
+        $this->assertArrayHasKey('error', $result);
+        $this->assertStringContainsString('out of FSWatcher journals', $result['error']);
+    }
+
+    private function prepareWpConfig()
+    {
+        $wp_config = ABSPATH . 'wp-config.php';
+        if (file_exists($wp_config)) {
+            $this->markTestSkipped('A real wp-config.php exists in ABSPATH, it is not touched by tests.');
+        }
+        file_put_contents($wp_config, "<?php // spbc-fsw-test-marker\n");
+        $this->files_to_remove[] = $wp_config;
+
+        return $wp_config;
+    }
+
+    private function makeTempFile($name)
+    {
+        $path = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'spbc-fsw-' . $name . '-' . uniqid() . '.php';
+        file_put_contents($path, "<?php // spbc-fsw-test-marker\n");
+        $this->files_to_remove[] = $path;
+
+        return $path;
+    }
+
+    private function writeJournals(array $first, array $second)
+    {
+        global $spbc;
+        $dir = FileStorage::getJournalsPath();
+        $key_hash = md5($spbc->settings['spbc_key']);
+        foreach (array(self::FIRST_JOURNAL => $first, self::SECOND_JOURNAL => $second) as $id => $rows) {
+            $csv = '';
+            foreach ($rows as $path => $mtime) {
+                $csv .= '"' . $path . '",' . $mtime . "\n";
+            }
+            $journal = $dir . $id . '_' . $key_hash . FileStorage::STATUS_COMPLETED . '.csv.gz';
+            $gz = gzopen($journal, 'w9');
+            gzwrite($gz, $csv);
+            gzclose($gz);
+            $this->files_to_remove[] = $journal;
+        }
+    }
+
+    /**
+     * @return array{data?: string, error?: string}
+     */
+    private function view($path)
+    {
+        $_POST = wp_slash(array(
+            'fswatcher_file_path' => $path,
+            'fswatcher__first_date' => (string) self::FIRST_JOURNAL,
+            'fswatcher__second_date' => (string) self::SECOND_JOURNAL,
+        ));
+
+        try {
+            return array('data' => Analyzer::getViewFile());
+        } catch (\Exception $e) {
+            return array('error' => $e->getMessage());
+        }
+    }
+}

From d000411fed89b7ac2eac4352672cb3b7925196a9 Mon Sep 17 00:00:00 2001
From: "github-actions[bot]" <github-actions[bot]@users.noreply.github.com>
Date: Tue, 27 Oct 2026 04:50:20 +0000
Subject: [PATCH 17/17] chore(changelog): rebuild readme for v2.191 [skip ci]

---
 changelog.txt | 2 ++
 readme.txt    | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/changelog.txt b/changelog.txt
index 405d071a2..54afb8469 100644
--- a/changelog.txt
+++ b/changelog.txt
@@ -1,6 +1,8 @@
 == Changelog ==
 
 = 2.191 Oct 22 2026 =
+* Fix. FSW. Restriction of access to FSW, edits for wpms, restriction for file actions (#762)
+* Fix. 2FA. Flow updates. (#761)
 * Fix. Settings. Links updated.
 * Fix. Cure. Fixed cure regex errors before writing file contents.
 * Fix. Scanner. Cure. Skip cure stage if option is currently disabled in settings.
diff --git a/readme.txt b/readme.txt
index 6b773afe3..f57fefa4b 100644
--- a/readme.txt
+++ b/readme.txt
@@ -377,6 +377,8 @@ Security FireWall can mitigate HTTP/HTTPS DDoS attacks. When an intruder makes G
 == Changelog ==
 
 = 2.191 Oct 22 2026 =
+* Fix. FSW. Restriction of access to FSW, edits for wpms, restriction for file actions (#762)
+* Fix. 2FA. Flow updates. (#761)
 * Fix. Settings. Links updated.
 * Fix. Cure. Fixed cure regex errors before writing file contents.
 * Fix. Scanner. Cure. Skip cure stage if option is currently disabled in settings.