diff --git a/composer.json b/composer.json index a195e0821..2407717c7 100644 --- a/composer.json +++ b/composer.json @@ -8,6 +8,9 @@ "email": "plugins@cleantalk.org" } ], + "require": { + "cleantalk/rate-limiter": "*" + }, "require-dev": { "vimeo/psalm": "^4.8", "phpunit/phpunit": "^8.5.52", diff --git a/lib/Cleantalk/ApbctWP/RateLimit/ApbctRateLimiter.php b/lib/Cleantalk/ApbctWP/RateLimit/ApbctRateLimiter.php new file mode 100644 index 000000000..a5605c798 --- /dev/null +++ b/lib/Cleantalk/ApbctWP/RateLimit/ApbctRateLimiter.php @@ -0,0 +1,178 @@ +ip = (string) Helper::ipGet('remote_addr', true); + } + + /** + * Sets the User-Agent from server variables + * + * @return void + */ + protected function setUA(): void + { + $this->ua = Server::getString('HTTP_USER_AGENT'); + } + + /** + * Override UID to be per-IP only (ignoring UA), + * so an attacker cannot bypass the limit by rotating User-Agent. + * + * @return void + */ + protected function setUID(): void + { + $this->uid = md5($this->ip . $this->config->type); + } + + /** + * Performs health check to ensure database table exists and config is valid + * + * @return bool + */ + protected function healthCheck(): bool + { + global $wpdb; + + if ( ! defined('APBCT_TBL_RATE_LIMITS') ) { + return false; + } + + $table_name = APBCT_TBL_RATE_LIMITS; + $sql = $wpdb->prepare('SHOW TABLES LIKE %s', $table_name); + $table_ok = ! empty($wpdb->get_var($sql)); + + return $table_ok && parent::healthCheck(); + } + + /** + * Handles rate limiter errors (silent fail - allow request through) + * + * @param string $msg Error message + * @return void + */ + protected function handleErrors(string $msg): void + { + // Silent fail - errors are handled by process_ok flag in the caller + } + + /** + * Retrieves rate limit data for the current UID from database + * + * @return RateLimiterDto|false + */ + protected function selectUIDData() + { + global $wpdb; + + $table_name = APBCT_TBL_RATE_LIMITS; + + $sql = $wpdb->prepare( + "SELECT uid, type, ip, ua, counter, last_call, created_at FROM {$table_name} WHERE uid = %s LIMIT 1", + $this->uid + ); + + $result = $wpdb->get_row($sql, ARRAY_A); + + return ! empty($result) ? new RateLimiterDto($result) : false; + } + + /** + * Inserts a new rate limit record + * + * @param RateLimiterDto $uid_data + * @return bool + */ + protected function insert(RateLimiterDto $uid_data): bool + { + global $wpdb; + + $table_name = APBCT_TBL_RATE_LIMITS; + + $sql = $wpdb->prepare( + "INSERT INTO {$table_name} (uid, type, ip, ua, counter, last_call, created_at) + VALUES (%s, %s, %s, %s, 1, %d, %d) + ON DUPLICATE KEY UPDATE last_call = %d, counter = counter + 1", + $uid_data->uid, + $uid_data->type, + $uid_data->ip, + $uid_data->ua, + $uid_data->last_call, + $uid_data->created_at, + $uid_data->last_call + ); + + return false !== $wpdb->query($sql); + } + + /** + * Increments the counter for an existing rate limit record. + * Resets counter if the period has expired. + * + * @param RateLimiterDto $uid_data + * @return bool + */ + protected function increment(RateLimiterDto $uid_data): bool + { + global $wpdb; + + $table_name = APBCT_TBL_RATE_LIMITS; + + $is_expired = ($this->current_ts - $uid_data->created_at) > $this->config->period; + + $uid_data->counter = $is_expired ? 1 : $uid_data->counter + 1; + $uid_data->created_at = $is_expired ? $this->current_ts : $uid_data->created_at; + $uid_data->last_call = $this->current_ts; + + $sql = $wpdb->prepare( + "UPDATE {$table_name} SET counter = %d, last_call = %d, created_at = %d WHERE uid = %s", + $uid_data->counter, + $uid_data->last_call, + $uid_data->created_at, + $uid_data->uid + ); + + return false !== $wpdb->query($sql); + } + + /** + * Removes expired rate limit records from the database + * + * @return bool + */ + protected function cleanUp(): bool + { + global $wpdb; + + $table_name = APBCT_TBL_RATE_LIMITS; + $threshold = $this->current_ts - ($this->config->period + 10); + + $sql = $wpdb->prepare( + "DELETE FROM {$table_name} WHERE created_at < %d AND type = %s", + $threshold, + $this->config->type + ); + + return false !== $wpdb->query($sql); + } +} diff --git a/lib/Cleantalk/ApbctWP/RemoteCalls.php b/lib/Cleantalk/ApbctWP/RemoteCalls.php index 363e49c64..3821d9e9f 100644 --- a/lib/Cleantalk/ApbctWP/RemoteCalls.php +++ b/lib/Cleantalk/ApbctWP/RemoteCalls.php @@ -3,10 +3,11 @@ namespace Cleantalk\ApbctWP; use Cleantalk\ApbctWP\Firewall\SFWUpdateHelper; -use Cleantalk\ApbctWP\UpdatePlugin\DbAnalyzer; +use Cleantalk\ApbctWP\RateLimit\ApbctRateLimiter; use Cleantalk\ApbctWP\Variables\Post; use Cleantalk\ApbctWP\Variables\Request; use Cleantalk\ApbctWP\Variables\Get; +use Cleantalk\Common\RateLimiter\RateLimiterConfig; use Cleantalk\Common\TT; class RemoteCalls @@ -102,6 +103,11 @@ public static function perform() $action = strtolower(Request::getString('spbc_remote_call_action')); $token = strtolower(Request::getString('spbc_remote_call_token')); + // Rate limit check — block abusive IPs before any cooldown logic + if ( ! self::rateLimitCheck() ) { + die('FAIL ' . json_encode(array('error' => 'RATE_LIMIT_EXCEEDED'))); + } + if ( isset($apbct->remote_calls[$action]) ) { $cooldown = isset($apbct->remote_calls[$action]['cooldown']) ? $apbct->remote_calls[$action]['cooldown'] : self::COOLDOWN; @@ -114,9 +120,6 @@ public static function perform() time() - $apbct->remote_calls[$action]['last_call'] >= $cooldown || ($action === 'sfw_update' && Request::get('file_urls')) ) { - $apbct->remote_calls[$action]['last_call'] = time(); - $apbct->save('remote_calls'); - if ( ! self::isRcAllowed() ) { die('FAIL ' . json_encode(array('error' => 'FORBIDDEN'))); } @@ -126,6 +129,10 @@ public static function perform() (self::checkToken($token)) || (self::isAllowedWithoutToken($action) && self::checkWithoutToken()) ) { + // Update last_call only for authorized requests + $apbct->remote_calls[$action]['last_call'] = time(); + $apbct->save('remote_calls'); + // Flag to let plugin know that Remote Call is running. $apbct->rc_running = true; @@ -677,4 +684,23 @@ private static function hideSensitiveData($data) } return $data; } + + /** + * Rate limit check for remote calls. + * Blocks abusive IPs that exceed 10 requests per 60 seconds. + * + * @return bool True if request is allowed, false if rate limited + */ + private static function rateLimitCheck() + { + $config = new RateLimiterConfig('rc_remote_call', 10, 60); + $limiter = new ApbctRateLimiter($config); + + // If rate limiter failed (e.g. table missing), allow the request through + if ( ! $limiter->process_ok ) { + return true; + } + + return $limiter->checkPassed(); + } } diff --git a/lib/Cleantalk/ApbctWP/State.php b/lib/Cleantalk/ApbctWP/State.php index 3d3e859c0..4c2fc7e97 100644 --- a/lib/Cleantalk/ApbctWP/State.php +++ b/lib/Cleantalk/ApbctWP/State.php @@ -531,6 +531,10 @@ protected function setDefinitions() // Table with session data. define('APBCT_SPAMSCAN_LOGS', $db_prefix . 'cleantalk_spamscan_logs'); } + if ( ! defined('APBCT_TBL_RATE_LIMITS')) { + // Table with rate limit data. + define('APBCT_TBL_RATE_LIMITS', $db_prefix . 'cleantalk_rate_limits'); + } if ( ! defined('APBCT_SELECT_LIMIT')) { // Select limit for logs. define('APBCT_SELECT_LIMIT', 5000); diff --git a/lib/Cleantalk/ApbctWP/UpdatePlugin/DbColumnCreator.php b/lib/Cleantalk/ApbctWP/UpdatePlugin/DbColumnCreator.php index 46290174f..3ae812be2 100644 --- a/lib/Cleantalk/ApbctWP/UpdatePlugin/DbColumnCreator.php +++ b/lib/Cleantalk/ApbctWP/UpdatePlugin/DbColumnCreator.php @@ -127,36 +127,42 @@ private function updateIndexes($schema_indexes_raw, $db_column_names, &$errors) $db_indexes_raw = $wpdb->get_results("SHOW INDEX FROM `$this->dbTableName`", ARRAY_A); if (is_string($schema_indexes_raw) && !empty($schema_indexes_raw)) { - // Parse index definitions from string like: "PRIMARY KEY (`id`), INDEX ( `network` , `mask` ), INDEX ( `status` )" - preg_match_all('/(?:PRIMARY\s+KEY|INDEX)\s*\([^)]+\)/i', $schema_indexes_raw, $matches); - if (isset($matches[0]) && !empty($matches[0])) { - foreach ($matches[0] as $match) { - // Extract column names from index definition - if (preg_match('/\(([^)]+)\)/', $match, $col_match) && isset($col_match[1])) { - $columns_raw = trim($col_match[1]); - $columns = preg_split('/\s*,\s*/', $columns_raw); - // Normalize column names - $normalized_columns = array(); - foreach ($columns as $col) { - $normalized_columns[] = trim($col, '` '); - } + // Parse index definitions from string like: "PRIMARY KEY (`id`), UNIQUE KEY `uid` (`uid`), INDEX (`type`)" + preg_match_all('/(PRIMARY\s+KEY|UNIQUE\s+(?:KEY|INDEX)|INDEX|KEY)\s*(?:`?(\w+)`?\s*)?\(([^)]+)\)/i', $schema_indexes_raw, $matches, PREG_SET_ORDER); + if (!empty($matches)) { + foreach ($matches as $match) { + if (!isset($match[1], $match[3])) { + continue; + } + $keyword = strtoupper(trim($match[1])); + $explicit_name = !empty($match[2]) ? $match[2] : ''; + $columns_raw = trim($match[3]); - // Skip PRIMARY KEY as it should already exist if table has primary key - if (stripos($match, 'PRIMARY') !== false) { - continue; - } + // Skip PRIMARY KEY as it should already exist if table has primary key + if (strpos($keyword, 'PRIMARY') !== false) { + continue; + } - if (!empty($normalized_columns)) { - $index_name = $normalized_columns[0]; + $columns = preg_split('/\s*,\s*/', $columns_raw); + // Normalize column names + $normalized_columns = array(); + foreach ($columns as $col) { + $normalized_columns[] = trim($col, '` '); + } - $normalized_def = 'INDEX (' . implode(',', array_map(function ($col) { - return '`' . $col . '`'; - }, $normalized_columns)) . ')'; + if (!empty($normalized_columns)) { + // Use explicit name if provided, otherwise first column name + $index_name = !empty($explicit_name) ? $explicit_name : $normalized_columns[0]; - // Use first column as index name - $schema_indexes[$index_name] = $normalized_def; - $schema_index_names[] = $index_name; - } + $is_unique = (strpos($keyword, 'UNIQUE') !== false); + $index_type = $is_unique ? 'UNIQUE INDEX' : 'INDEX'; + + $normalized_def = $index_type . ' (' . implode(',', array_map(function ($col) { + return '`' . $col . '`'; + }, $normalized_columns)) . ')'; + + $schema_indexes[$index_name] = $normalized_def; + $schema_index_names[] = $index_name; } } } @@ -251,7 +257,8 @@ private function updateIndexes($schema_indexes_raw, $db_column_names, &$errors) $index_def = $schema_indexes[$index_name]; if (preg_match('/\(([^)]+)\)/', $index_def, $col_match) && isset($col_match[1])) { $columns = trim($col_match[1]); - $sql = "ALTER TABLE `$this->dbTableName` ADD INDEX `$index_name` ($columns)"; + $add_type = (stripos($index_def, 'UNIQUE') !== false) ? 'UNIQUE INDEX' : 'INDEX'; + $sql = "ALTER TABLE `$this->dbTableName` ADD $add_type `$index_name` ($columns)"; $result = $wpdb->query($sql); if ($result === false) { $errors[] = "Failed to recreate index `$index_name`.\nQuery: $wpdb->last_query\nError: $wpdb->last_error"; @@ -287,7 +294,8 @@ private function updateIndexes($schema_indexes_raw, $db_column_names, &$errors) // Parse the index definition to extract columns if (preg_match('/\(([^)]+)\)/', $index_def, $col_match) && isset($col_match[1])) { $columns = trim($col_match[1]); - $sql = "ALTER TABLE `$this->dbTableName` ADD INDEX `$diff_index_name` ($columns)"; + $add_type = (stripos($index_def, 'UNIQUE') !== false) ? 'UNIQUE INDEX' : 'INDEX'; + $sql = "ALTER TABLE `$this->dbTableName` ADD $add_type `$diff_index_name` ($columns)"; $result = $wpdb->query($sql); if ($result === false) { $errors[] = "Failed to add index `$diff_index_name`.\nQuery: $wpdb->last_query\nError: $wpdb->last_error"; diff --git a/lib/Cleantalk/Common/RateLimiter/.gitignore b/lib/Cleantalk/Common/RateLimiter/.gitignore new file mode 100644 index 000000000..cc341b5c2 --- /dev/null +++ b/lib/Cleantalk/Common/RateLimiter/.gitignore @@ -0,0 +1,4 @@ +.idea +/vendor/ +/composer.lock +/tests/.phpunit.result.cache diff --git a/lib/Cleantalk/Common/RateLimiter/README.md b/lib/Cleantalk/Common/RateLimiter/README.md new file mode 100644 index 000000000..5ab804db9 --- /dev/null +++ b/lib/Cleantalk/Common/RateLimiter/README.md @@ -0,0 +1 @@ +How to use this app here diff --git a/lib/Cleantalk/Common/RateLimiter/RateLimiter.php b/lib/Cleantalk/Common/RateLimiter/RateLimiter.php new file mode 100644 index 000000000..7f731f246 --- /dev/null +++ b/lib/Cleantalk/Common/RateLimiter/RateLimiter.php @@ -0,0 +1,227 @@ +config = $config; + $this->current_ts = time(); + + $this->setIP(); + $this->setUA(); + $this->setUID(); + } + + /** + * Sets the unique identifier for the current request + * Must be implemented by child classes + * + * @return void + */ + protected function setUID(): void + { + $this->uid = md5($this->ip . $this->ua . $this->config->type); + } + + /** + * Determines if the current request should be rate limited + * + * @return bool True if request should be allowed or error occurred, false if rate limited + */ + public function checkPassed(): bool + { + try { + if (!$this->healthCheck()) { + throw new \Exception('HEALTH_CHECK_FAILED'); + } + + if (!$this->cleanUp()) { + throw new \Exception('CLEANUP_FAILED'); + } + + $uid_data = $this->selectUIDData(); + $record_found = false !== $uid_data; + + if ($record_found && !$uid_data->data_ok) { + throw new \Exception('UID_DATA_INVALID'); + } + + if ($record_found && $this->isLocked($uid_data)) { + return false; // Block here by limit exceeded + } + + if ($record_found) { + if (!$this->increment($uid_data)) { + throw new \Exception('INCREMENT_FAILED'); + } + } else { + $uid_data = new RateLimiterDto( + array( + 'uid' => $this->uid, + 'type' => $this->config->type, + 'counter' => 1, + 'last_call' => $this->current_ts, + 'created_at' => $this->current_ts, + 'ip' => $this->ip, + 'ua' => $this->ua, + ) + ); + if (!$uid_data->data_ok) { + throw new \Exception('UID_DATA_INVALID__INSERT'); + } + if (!$this->insert($uid_data)) { + throw new \Exception('INSERT_FAILED'); + } + } + } catch (\Exception $e) { + $this->process_ok = false; + $this->handleErrors($e->getMessage()); + return true; + } + + return true; + } + + /** + * Checks if the current UID has exceeded the rate limit + * + * @param RateLimiterDto $uid_data + * @return bool True if rate limited, false otherwise + */ + protected function isLocked(RateLimiterDto $uid_data): bool + { + return $uid_data->data_ok && ($uid_data->counter > $this->config->limit); + } + + /** + * Performs basic health check on configuration + * + * @return bool True if configuration is valid, false otherwise + */ + protected function healthCheck(): bool + { + if ($this->config->limit < 1) { + return false; + } + if ($this->config->period < 1) { + return false; + } + if ($this->config->type === null) { + return false; + } + return true; + } + + /** + * Retrieves rate limit data for the current UID + * Default implementation returns empty data + * + * @return RateLimiterDto|false Rate limit data or false if not found + */ + protected function selectUIDData() + { + return new RateLimiterDto(array()); + } + + /** + * Sets the IP address for the current request + * Must be implemented by child classes + * + * @return void + */ + abstract protected function setIP(): void; + + /** + * Sets the IP address for the current request + * Must be implemented by child classes + * + * @return void + */ + abstract protected function setUA(): void; + + /** + * Handles errors that occur during rate limiting + * Must be implemented by child classes + * + * @param string $msg Error message + * @return void + */ + abstract protected function handleErrors(string $msg): void; + + /** + * Increments the counter for an existing rate limit record + * Must be implemented by child classes + * @param RateLimiterDto $uid_data + * @return bool True on success, false on failure + */ + abstract protected function increment(RateLimiterDto $uid_data): bool; + + /** + * Inserts a new rate limit record + * Must be implemented by child classes + * @param RateLimiterDto $uid_data + * @return bool True on success, false on failure + */ + abstract protected function insert(RateLimiterDto $uid_data): bool; + + /** + * Cleans up expired rate limit records + * Must be implemented by child classes + * + * @return bool True on success, false on failure + */ + abstract protected function cleanUp(): bool; +} diff --git a/lib/Cleantalk/Common/RateLimiter/RateLimiterConfig.php b/lib/Cleantalk/Common/RateLimiter/RateLimiterConfig.php new file mode 100644 index 000000000..93f673b38 --- /dev/null +++ b/lib/Cleantalk/Common/RateLimiter/RateLimiterConfig.php @@ -0,0 +1,50 @@ +type = $type; + $this->limit = $limit; + $this->period = $period; + } +} diff --git a/lib/Cleantalk/Common/RateLimiter/RateLimiterDto.php b/lib/Cleantalk/Common/RateLimiter/RateLimiterDto.php new file mode 100644 index 000000000..59a83726b --- /dev/null +++ b/lib/Cleantalk/Common/RateLimiter/RateLimiterDto.php @@ -0,0 +1,94 @@ +data_ok = true; + } catch (\Exception $_e) { + $this->data_ok = false; + } + } +} diff --git a/lib/Cleantalk/Common/RateLimiter/composer.json b/lib/Cleantalk/Common/RateLimiter/composer.json new file mode 100644 index 000000000..422de8331 --- /dev/null +++ b/lib/Cleantalk/Common/RateLimiter/composer.json @@ -0,0 +1,39 @@ +{ + "name": "cleantalk/rate-limiter", + "description": "CleanTalk APBCT RateLimit classes", + "authors": [ + { + "name": "CleanTalk Team", + "email": "support@cleantalk.org" + } + ], + "license": "GPL-3.0-or-later", + "autoload": { + "psr-4": { + "Cleantalk\\Common\\RateLimiter\\": "./" + } + }, + "require": { + "cleantalk/templates": "*" + }, + "require-dev": { + "vimeo/psalm": "^4.8", + "phpunit/phpunit": "^8.5.52", + "squizlabs/php_codesniffer": "3.*", + "phpcompatibility/php-compatibility": "^9.3" + }, + "config": { + "allow-plugins": { + "dealerdirect/phpcodesniffer-composer-installer": true + } + }, + "scripts": { + "test": [ + "vendor/bin/phpunit --configuration tests/phpunit.xml", + "vendor/bin/phpcs --config-set installed_paths vendor/phpcompatibility/php-compatibility", + "vendor/bin/phpcs --standard=tests/.phpcs.xml", + "vendor/bin/psalm --no-cache --config=tests/psalm.xml", + "vendor/bin/psalm --no-cache --config=tests/psalm.xml --taint-analysis" + ] + } +} diff --git a/lib/Cleantalk/Common/RateLimiter/tests/.phpcs.xml b/lib/Cleantalk/Common/RateLimiter/tests/.phpcs.xml new file mode 100644 index 000000000..ca06694be --- /dev/null +++ b/lib/Cleantalk/Common/RateLimiter/tests/.phpcs.xml @@ -0,0 +1,30 @@ + + + Sniff code to check different PHP compatibility an PSR-12 Code Style + + + ../ + + + lib/* + tests/* + vendor/* + node_modules/* + + + + + + + + + + + + + + + + + + diff --git a/lib/Cleantalk/Common/RateLimiter/tests/phpunit.xml b/lib/Cleantalk/Common/RateLimiter/tests/phpunit.xml new file mode 100644 index 000000000..df01333e0 --- /dev/null +++ b/lib/Cleantalk/Common/RateLimiter/tests/phpunit.xml @@ -0,0 +1,16 @@ + + + + + ./ + ./bootstrap.php + + + diff --git a/lib/Cleantalk/Common/RateLimiter/tests/psalm.xml b/lib/Cleantalk/Common/RateLimiter/tests/psalm.xml new file mode 100644 index 000000000..28a40a3e9 --- /dev/null +++ b/lib/Cleantalk/Common/RateLimiter/tests/psalm.xml @@ -0,0 +1,29 @@ + + + + + + + + + + + + + diff --git a/lib/Cleantalk/Common/Schema.php b/lib/Cleantalk/Common/Schema.php index 0151d006a..0d2788b7a 100644 --- a/lib/Cleantalk/Common/Schema.php +++ b/lib/Cleantalk/Common/Schema.php @@ -100,6 +100,18 @@ class Schema '__indexes' => 'PRIMARY KEY (`id`)', '__createkey' => 'INT unsigned primary KEY AUTO_INCREMENT FIRST' ), + 'rate_limits' => array( + 'id' => 'INT NOT NULL AUTO_INCREMENT', + 'uid' => 'VARCHAR(32) NOT NULL', + 'type' => 'VARCHAR(64) NOT NULL', + 'ip' => 'VARCHAR(45) NOT NULL', + 'ua' => 'VARCHAR(512) NOT NULL DEFAULT \'\'', + 'counter' => 'INT NOT NULL DEFAULT 0', + 'last_call' => 'INT NOT NULL DEFAULT 0', + 'created_at' => 'INT NOT NULL DEFAULT 0', + '__indexes' => 'PRIMARY KEY (`id`), UNIQUE KEY `uid` (`uid`), INDEX (`type`), INDEX (`created_at`)', + '__createkey' => 'INT unsigned primary KEY AUTO_INCREMENT FIRST' + ), ); /** diff --git a/lib/Cleantalk/Common/Templates/.gitignore b/lib/Cleantalk/Common/Templates/.gitignore new file mode 100644 index 000000000..cb424410f --- /dev/null +++ b/lib/Cleantalk/Common/Templates/.gitignore @@ -0,0 +1,3 @@ +/.idea/ +/vendor/ +/composer.lock \ No newline at end of file diff --git a/lib/Cleantalk/Common/Templates/Dto.php b/lib/Cleantalk/Common/Templates/Dto.php new file mode 100644 index 000000000..482837b2a --- /dev/null +++ b/lib/Cleantalk/Common/Templates/Dto.php @@ -0,0 +1,59 @@ +isObligatoryParamsPresented($params) ) { + throw new \Exception('No go!'); + } + + foreach ( $params as $param_name => $param ) { + if ( property_exists(static::class, $param_name) ) { + $type = gettype($this->$param_name); + $this->$param_name = $param; + settype($this->$param_name, $type); + } + } + } + + /** + * @param $params + * + * @return bool + * @since 1.1.0 + * + */ + private function isObligatoryParamsPresented($params) + { + return empty($this->obligatory_properties) || + count(array_intersect($this->obligatory_properties, array_keys($params))) === count( + $this->obligatory_properties + ); + } + + /** + * Get array of all DTO properties as key-value, except obligatory_properties. + * @return array + */ + public function getArray() + { + $array = array(); + foreach (get_object_vars($this) as $key => $value) { + $array[$key] = $value; + } + unset($array['obligatory_properties']); + return $array; + } +} diff --git a/lib/Cleantalk/Common/Templates/Multiton.php b/lib/Cleantalk/Common/Templates/Multiton.php new file mode 100644 index 000000000..7fef9b70b --- /dev/null +++ b/lib/Cleantalk/Common/Templates/Multiton.php @@ -0,0 +1,28 @@ +init($params); + } + + return static::$instances[$instance]; + } +} diff --git a/lib/Cleantalk/Common/Templates/Singleton.php b/lib/Cleantalk/Common/Templates/Singleton.php new file mode 100644 index 000000000..e21581a72 --- /dev/null +++ b/lib/Cleantalk/Common/Templates/Singleton.php @@ -0,0 +1,47 @@ +init($params); + } + return static::$instance; + } + + /** + * Alternative constructor + */ + protected function init($params = array()) + { + } + + public static function resetInstance() + { + static::$instance = null; + } + } +} diff --git a/lib/Cleantalk/Common/Templates/composer.json b/lib/Cleantalk/Common/Templates/composer.json new file mode 100644 index 000000000..5f406249e --- /dev/null +++ b/lib/Cleantalk/Common/Templates/composer.json @@ -0,0 +1,16 @@ +{ + "name": "cleantalk/templates", + "description": "CleanTalk APBCT traits", + "authors": [ + { + "name": "CleanTalk Team", + "email": "support@cleantalk.org" + } + ], + "license": "GPL-3.0-or-later", + "autoload": { + "psr-4": { + "Cleantalk\\Common\\Templates\\": "./" + } + } +} diff --git a/psalm.xml b/psalm.xml index 64d954f0c..ba15ede07 100644 --- a/psalm.xml +++ b/psalm.xml @@ -26,6 +26,9 @@ + + + diff --git a/tests/ApbctWP/RateLimit/TestApbctRateLimiter.php b/tests/ApbctWP/RateLimit/TestApbctRateLimiter.php new file mode 100644 index 000000000..e1079a91c --- /dev/null +++ b/tests/ApbctWP/RateLimit/TestApbctRateLimiter.php @@ -0,0 +1,256 @@ +test_ip = $ip; + $this->test_ua = $ua; + parent::__construct($config); + } + + protected function setIP(): void + { + $this->ip = $this->test_ip; + } + + protected function setUA(): void + { + $this->ua = $this->test_ua; + } + + /** + * Same override as ApbctRateLimiter — per-IP only, ignoring UA. + */ + protected function setUID(): void + { + $this->uid = md5($this->ip . $this->config->type); + } + + protected function handleErrors(string $msg): void + { + } + + protected function increment(RateLimiterDto $uid_data): bool + { + return true; + } + + protected function insert(RateLimiterDto $uid_data): bool + { + return true; + } + + protected function cleanUp(): bool + { + return true; + } + + // --- Expose protected properties for assertions --- + + public function getUID(): string + { + return $this->uid; + } + + public function getIP(): string + { + return $this->ip; + } + + public function getUA(): string + { + return $this->ua; + } +} + +/** + * Stub with the DEFAULT base RateLimiter::setUID() (ip + ua + type) + * to demonstrate the difference with the per-IP override. + */ +class TestRateLimiterBaseStub extends RateLimiter +{ + private $test_ip; + private $test_ua; + + public function __construct(RateLimiterConfig $config, string $ip, string $ua) + { + $this->test_ip = $ip; + $this->test_ua = $ua; + parent::__construct($config); + } + + protected function setIP(): void + { + $this->ip = $this->test_ip; + } + + protected function setUA(): void + { + $this->ua = $this->test_ua; + } + + // No setUID override — uses base RateLimiter logic: md5(ip + ua + type) + + protected function handleErrors(string $msg): void + { + } + + protected function increment(RateLimiterDto $uid_data): bool + { + return true; + } + + protected function insert(RateLimiterDto $uid_data): bool + { + return true; + } + + protected function cleanUp(): bool + { + return true; + } + + public function getUID(): string + { + return $this->uid; + } +} + +class TestApbctRateLimiter extends TestCase +{ + /** + * @test + * UID must equal md5(ip + type), without UA component + */ + public function uidMatchesExpectedHashOfIpAndType() + { + $config = new RateLimiterConfig('rc_remote_call', 10, 60); + $limiter = new TestRateLimiterStub($config, '192.168.1.1', 'Mozilla/5.0'); + + $expected = md5('192.168.1.1' . 'rc_remote_call'); + $this->assertSame($expected, $limiter->getUID()); + } + + /** + * @test + * Same IP with different User-Agents MUST produce the same UID (per-IP blocking). + * This is the core security fix — attacker cannot bypass rate limit by rotating UA. + */ + public function sameIpDifferentUaProducesSameUid() + { + $config = new RateLimiterConfig('rc_remote_call', 10, 60); + + $limiter1 = new TestRateLimiterStub($config, '10.0.0.1', 'Mozilla/5.0'); + $limiter2 = new TestRateLimiterStub($config, '10.0.0.1', 'curl/7.68.0'); + $limiter3 = new TestRateLimiterStub($config, '10.0.0.1', ''); + + $this->assertSame($limiter1->getUID(), $limiter2->getUID()); + $this->assertSame($limiter1->getUID(), $limiter3->getUID()); + } + + /** + * @test + * Different IPs must produce different UIDs — rate limit is per-IP. + */ + public function differentIpsProduceDifferentUids() + { + $config = new RateLimiterConfig('rc_remote_call', 10, 60); + + $limiter1 = new TestRateLimiterStub($config, '10.0.0.1', 'Mozilla/5.0'); + $limiter2 = new TestRateLimiterStub($config, '10.0.0.2', 'Mozilla/5.0'); + + $this->assertNotSame($limiter1->getUID(), $limiter2->getUID()); + } + + /** + * @test + * Different config types produce different UIDs for the same IP. + */ + public function differentTypesProduceDifferentUids() + { + $config1 = new RateLimiterConfig('rc_remote_call', 10, 60); + $config2 = new RateLimiterConfig('login_attempt', 5, 300); + + $limiter1 = new TestRateLimiterStub($config1, '10.0.0.1', 'Mozilla/5.0'); + $limiter2 = new TestRateLimiterStub($config2, '10.0.0.1', 'Mozilla/5.0'); + + $this->assertNotSame($limiter1->getUID(), $limiter2->getUID()); + } + + /** + * @test + * Verify that the base RateLimiter (without override) DOES include UA in UID, + * while our override does NOT. This proves the override changes behavior. + */ + public function overrideExcludesUaWhileBaseIncludes() + { + $config = new RateLimiterConfig('rc_remote_call', 10, 60); + + // Base: same IP + different UA → different UID + $base1 = new TestRateLimiterBaseStub($config, '10.0.0.1', 'Mozilla/5.0'); + $base2 = new TestRateLimiterBaseStub($config, '10.0.0.1', 'curl/7.68.0'); + $this->assertNotSame($base1->getUID(), $base2->getUID(), 'Base UID should include UA'); + + // Override: same IP + different UA → SAME UID + $override1 = new TestRateLimiterStub($config, '10.0.0.1', 'Mozilla/5.0'); + $override2 = new TestRateLimiterStub($config, '10.0.0.1', 'curl/7.68.0'); + $this->assertSame($override1->getUID(), $override2->getUID(), 'Override UID should ignore UA'); + } + + /** + * @test + * UID is always a 32-char hex string (MD5 hash). + */ + public function uidIsValidMd5Hash() + { + $config = new RateLimiterConfig('rc_remote_call', 10, 60); + $limiter = new TestRateLimiterStub($config, '192.168.1.100', 'SomeAgent'); + + $uid = $limiter->getUID(); + $this->assertSame(32, strlen($uid)); + $this->assertRegExp('/^[0-9a-f]{32}$/', $uid); + } + + /** + * @test + * IPv6 address should also work correctly for UID generation. + */ + public function uidWorksWithIpv6() + { + $config = new RateLimiterConfig('rc_remote_call', 10, 60); + + $limiter1 = new TestRateLimiterStub($config, '::1', 'Mozilla/5.0'); + $limiter2 = new TestRateLimiterStub($config, '::1', 'curl/7.68.0'); + + $expected = md5('::1' . 'rc_remote_call'); + $this->assertSame($expected, $limiter1->getUID()); + $this->assertSame($limiter1->getUID(), $limiter2->getUID()); + } + + /** + * @test + * Empty IP edge case — should still produce a deterministic UID. + */ + public function uidWithEmptyIp() + { + $config = new RateLimiterConfig('rc_remote_call', 10, 60); + + $limiter = new TestRateLimiterStub($config, '', 'Mozilla/5.0'); + + $expected = md5('' . 'rc_remote_call'); + $this->assertSame($expected, $limiter->getUID()); + } +} diff --git a/tests/ApbctWP/TestRemoteCalls.php b/tests/ApbctWP/TestRemoteCalls.php index 5d84ede20..60873d4ca 100644 --- a/tests/ApbctWP/TestRemoteCalls.php +++ b/tests/ApbctWP/TestRemoteCalls.php @@ -381,4 +381,99 @@ public function itHidesNestedApiKeyWithUnderscore() $result['network_settings']['multisite__hoster_api_key'] ); } + + // ========================================================================= + // Rate limiting for remote calls + // ========================================================================= + + /** @test */ + public function rateLimitCheckMethodExists() + { + $this->assertTrue( + method_exists(RemoteCalls::class, 'rateLimitCheck'), + 'RemoteCalls must have a rateLimitCheck method' + ); + } + + /** @test */ + public function rateLimitCheckIsPrivateStatic() + { + $method = new ReflectionMethod(RemoteCalls::class, 'rateLimitCheck'); + $this->assertTrue($method->isPrivate(), 'rateLimitCheck must be private'); + $this->assertTrue($method->isStatic(), 'rateLimitCheck must be static'); + } + + /** @test */ + public function rateLimitCheckUsesCorrectConfig() + { + // Verify the config values used inside rateLimitCheck by inspecting the method body. + // The method creates RateLimiterConfig('rc_remote_call', 10, 60). + $method = new ReflectionMethod(RemoteCalls::class, 'rateLimitCheck'); + $method->setAccessible(true); + + // Read the source to confirm config values + $filename = $method->getFileName(); + $startLine = $method->getStartLine(); + $endLine = $method->getEndLine(); + $source = implode('', array_slice(file($filename), $startLine - 1, $endLine - $startLine + 1)); + + $this->assertStringContainsString("'rc_remote_call'", $source, 'Rate limit type must be rc_remote_call'); + $this->assertStringContainsString('10', $source, 'Rate limit must be set to 10 requests'); + $this->assertStringContainsString('60', $source, 'Rate limit period must be 60 seconds'); + } + + /** @test */ + public function performCallsRateLimitCheckBeforeCooldown() + { + // Verify that rateLimitCheck() is called BEFORE any cooldown logic in perform(). + // Read perform() source and check ordering. + $method = new ReflectionMethod(RemoteCalls::class, 'perform'); + $filename = $method->getFileName(); + $startLine = $method->getStartLine(); + $endLine = $method->getEndLine(); + $source = implode('', array_slice(file($filename), $startLine - 1, $endLine - $startLine + 1)); + + $rateLimitPos = strpos($source, 'rateLimitCheck'); + $cooldownPos = strpos($source, 'last_call'); + + $this->assertNotFalse($rateLimitPos, 'perform() must call rateLimitCheck'); + $this->assertNotFalse($cooldownPos, 'perform() must reference last_call'); + $this->assertLessThan($cooldownPos, $rateLimitPos, 'rateLimitCheck must be called BEFORE last_call / cooldown logic'); + } + + /** @test */ + public function lastCallIsUpdatedAfterTokenCheck() + { + // Verify that last_call is updated AFTER checkToken / isAllowedWithoutToken, + // not before. This prevents attackers from updating cooldown with bogus tokens. + $method = new ReflectionMethod(RemoteCalls::class, 'perform'); + $filename = $method->getFileName(); + $startLine = $method->getStartLine(); + $endLine = $method->getEndLine(); + $source = implode('', array_slice(file($filename), $startLine - 1, $endLine - $startLine + 1)); + + $checkTokenPos = strpos($source, 'checkToken'); + $lastCallUpdate = strpos($source, "['last_call'] = time()"); + + $this->assertNotFalse($checkTokenPos, 'perform() must call checkToken'); + $this->assertNotFalse($lastCallUpdate, 'perform() must update last_call'); + $this->assertGreaterThan( + $checkTokenPos, + $lastCallUpdate, + 'last_call update must come AFTER token validation' + ); + } + + /** @test */ + public function performDiesWithRateLimitErrorWhenBlocked() + { + // Verify the error response format for rate-limited requests + $method = new ReflectionMethod(RemoteCalls::class, 'perform'); + $filename = $method->getFileName(); + $startLine = $method->getStartLine(); + $endLine = $method->getEndLine(); + $source = implode('', array_slice(file($filename), $startLine - 1, $endLine - $startLine + 1)); + + $this->assertStringContainsString('RATE_LIMIT_EXCEEDED', $source, 'perform() must use RATE_LIMIT_EXCEEDED error code'); + } }