Merge branch 'release-0.24.0'

chesio · Jul 29, 2024 · c1b321d · c1b321d
2 parents 0122d94 + e618e09
commit c1b321d
Show file tree

Hide file tree

Showing 17 changed files with 274 additions and 225 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,19 @@
 # BC Security Changelog
 
+## Version 0.24.0 (2024-07-29)
+
+WordPress 6.4 or newer is now required!
+
+### Added
+
+* Disable autoloading of plugin options when plugin is deactivated [#160](https://github.com/chesio/bc-security/issues/160).
+* New built-in rule for bad request banner module that triggers when non-existing `.asp` or `.aspx` file is accessed [#161](https://github.com/chesio/bc-security/issues/161).
+* Plugin has been tested with WordPress 6.6 [#157](https://github.com/chesio/bc-security/issues/157).
+
+### Changed
+
+* WordPress 6.4 is required [#159](https://github.com/chesio/bc-security/issues/159).
+
 ## Version 0.23.0 (2024-04-04)
 
 **Important**: either deactivate and reactivate plugin after update or install new cron job manually via WP-CLI: `wp cron event schedule bc-security/failed-logins-clean-up now daily`.

diff --git a/README.md b/README.md
@@ -10,7 +10,7 @@ A WordPress plugin that helps keeping WordPress websites secure.
 ## Requirements
 
 * [PHP](https://www.php.net/) 8.1 or newer
-* [WordPress](https://wordpress.org/) 6.2 or newer
+* [WordPress](https://wordpress.org/) 6.4 or newer
 
 ## Limitations
 
@@ -144,11 +144,12 @@ Passwords are validated on user creation, password change or password reset. If
 
 Remote IP addresses that are scanning your website for weaknesses can be automatically [blocked](#internal-blocklist) for configured amount of time. Such scanners can be usually quite easily detected because while scanning a website they trigger a lot of 404 errors and URLs they try to access differ from "valid" 404 errors: usually they try to find a known vulnerable plugin, forgotten backup file or PHP script used for administrative purposes.
 
-There are four built-in rules available (they are not active by default):
-1. ban when non-existent PHP file is requested (any URL ending with `.php`)
-2. ban when non-existent archive file is requested (any URL ending with `.tgz` or `.zip`)
-3. ban when non-existent backup file is requested (any URL targeting file with `backup` in basename or with `.back`, `.old` or `.tmp` extension)
-4. ban when non-existent `readme.txt` file is accessed
+There are five built-in rules available (they are not active by default):
+1. ban when non-existent APS file is requested (any URL ending with `.asp` or `.aspx`)
+2. ban when non-existent PHP file is requested (any URL ending with `.php`)
+3. ban when non-existent archive file is requested (any URL ending with `.tgz` or `.zip`)
+4. ban when non-existent backup file is requested (any URL targeting file with `backup` in basename or with `.back`, `.old` or `.tmp` extension)
+5. ban when non-existent `readme.txt` file is accessed
 
 You may define custom rules as well (in form of regular expression).
 

diff --git a/bc-security.php b/bc-security.php
@@ -4,12 +4,12 @@
  * Plugin Name: BC Security
  * Plugin URI: https://github.com/chesio/bc-security
  * Description: Helps keeping WordPress websites secure.
- * Version: 0.23.0
+ * Version: 0.24.0
  * Author: Česlav Przywara <[email protected]>
  * Author URI: https://www.chesio.com
  * Requires PHP: 8.1
- * Requires at least: 6.2
- * Tested up to: 6.5
+ * Requires at least: 6.4
+ * Tested up to: 6.6
  * Text Domain: bc-security
  * GitHub Plugin URI: https://github.com/chesio/bc-security
  * Update URI: https://github.com/chesio/bc-security

diff --git a/classes/BlueChip/Security/Core/Admin/SettingsPage.php b/classes/BlueChip/Security/Core/Admin/SettingsPage.php
@@ -63,7 +63,15 @@ public function displaySettingsErrors(): void
      */
     public function registerSettings(): void
     {
-        register_setting($this->option_group, $this->option_name, [$this->settings, 'sanitize']);
+        register_setting(
+            $this->option_group,
+            $this->option_name,
+            [
+                'type' => 'array',
+                'sanitize_callback' => $this->settings->sanitize(...),
+                'default' => $this->settings->getDefaultValue(),
+            ]
+        );
     }
 
 
@@ -105,7 +113,7 @@ protected function getFieldBaseProperties(string $key, $value = null): array
             'label_for' => \sprintf('%s-%s', $this->option_name, $key), // "label_for" is WP reserved name
             'key' => $key,
             'name' => \sprintf('%s[%s]', $this->option_name, $key),
-            'value' => null === $value ? $this->settings[$key] : $value,
+            'value' => $value ?? $this->settings[$key],
         ];
     }
 

diff --git a/classes/BlueChip/Security/Core/Settings.php b/classes/BlueChip/Security/Core/Settings.php
@@ -5,9 +5,6 @@
 namespace BlueChip\Security\Core;
 
 use ArrayAccess;
-use ArrayIterator;
-use IteratorAggregate;
-use Traversable;
 
 /**
  * Basis (abstract) class for setting objects.
@@ -17,9 +14,8 @@
  * @link https://developer.wordpress.org/plugins/settings/settings-api/
  *
  * @phpstan-implements ArrayAccess<string,mixed>
- * @phpstan-implements IteratorAggregate<string,mixed>
  */
-abstract class Settings implements ArrayAccess, IteratorAggregate
+abstract class Settings implements ArrayAccess
 {
     /**
      * @var array<string,mixed> Default values for all settings. Descendant classes should override it.
@@ -35,7 +31,7 @@ abstract class Settings implements ArrayAccess, IteratorAggregate
     /**
      * @var array<string,mixed> Settings data (kind of cache for get_option() result).
      */
-    protected array $data;
+    private array $data;
 
 
     /**
@@ -48,40 +44,6 @@ public function __construct(private string $option_name)
     }
 
 
-    /**
-     * Get value of setting under key $name.
-     *
-     * @param string $name
-     *
-     * @return mixed A null value is returned if $name is not a valid key.
-     */
-    public function __get(string $name): mixed
-    {
-        if (isset($this->data[$name])) {
-            return $this->data[$name];
-        } else {
-            _doing_it_wrong(__METHOD__, \sprintf('Unknown settings key "%s"', $name), '0.1.0');
-            return null;
-        }
-    }
-
-
-    /**
-     * Set value of setting under key $name to $value.
-     *
-     * @param string $name
-     * @param mixed $value
-     */
-    public function __set(string $name, mixed $value): void
-    {
-        if (isset($this->data[$name])) {
-            $this->update($name, $value);
-        } else {
-            _doing_it_wrong(__METHOD__, \sprintf('Unknown settings key "%s"', $name), '0.1.0');
-        }
-    }
-
-
     //// ArrayAccess API ///////////////////////////////////////////////////////
 
     /**
@@ -91,7 +53,7 @@ public function __set(string $name, mixed $value): void
      */
     public function offsetExists(mixed $offset): bool
     {
-        return isset($this->data[$offset]);
+        return $this->offsetGet($offset) !== null;
     }
 
 
@@ -106,7 +68,7 @@ public function offsetExists(mixed $offset): bool
      */
     public function offsetGet(mixed $offset): mixed
     {
-        return isset($this->data[$offset]) ? $this->data[$offset] : null;
+        return $this->data[$offset] ?? null;
     }
 
 
@@ -117,7 +79,23 @@ public function offsetGet(mixed $offset): mixed
      */
     public function offsetSet(mixed $offset, mixed $value): void
     {
-        $this->update($offset, $value);
+        $data = $this->get();
+
+        if (!isset($data[$offset])) {
+            // Cannot update, invalid setting name.
+            return;
+        }
+
+        if (null === $value) {
+            // Null value unsets (resets) setting to default state
+            unset($data[$offset]);
+        } else {
+            // Any other value updates it
+            $data[$offset] = $value;
+        }
+
+        // Use set() here to have the data sanitized properly.
+        $this->set($data);
     }
 
 
@@ -128,15 +106,20 @@ public function offsetSet(mixed $offset, mixed $value): void
      */
     public function offsetUnset(mixed $offset): void
     {
-        $this->update($offset, null);
+        $this->offsetSet($offset, null);
     }
 
 
-    //// IteratorAggregate API /////////////////////////////////////////////////
+    //// Public API ////////////////////////////////////////////////////////////
 
-    public function getIterator(): Traversable
+    /**
+     * Get default value for settings.
+     *
+     * @return array<string,mixed>
+     */
+    public function getDefaultValue(): array
     {
-        return new ArrayIterator($this->data);
+        return static::DEFAULTS;
     }
 
 
@@ -163,28 +146,35 @@ public function get(): array
 
 
     /**
-     * Set $data as option data.
+     * Sanitize $data and set them as option value.
      *
      * @param array<string,mixed> $data
      *
      * @return bool
      */
     public function set(array $data): bool
     {
-        $this->data = $this->sanitize($data);
-        return $this->persist();
+        return $this->persist($this->sanitize($data));
     }
 
 
     /**
-     * Reset option data.
+     * Reset option data to default values.
      *
      * @return bool
      */
     public function reset(): bool
     {
-        $this->data = static::DEFAULTS;
-        return $this->persist();
+        return $this->persist(static::DEFAULTS);
+    }
+
+
+    /**
+     * Set autoload value of underlying option to $autoload.
+     */
+    public function setAutoload(bool $autoload): bool
+    {
+        return wp_set_option_autoload($this->option_name, $autoload);
     }
 
 
@@ -200,16 +190,26 @@ public function destroy(): bool
 
 
     /**
-     * Persist the value of data into database.
+     * Set the value of $data into local cache and also persist it in database.
+     *
+     * @internal This function does no sanitization and thus is private - use set() or reset() in external code.
+     *
+     * @param array<string,mixed> $data Sanitized (!) data to persist.
      *
      * @return bool True if settings have been updated (= changed), false otherwise.
      */
-    public function persist(): bool
+    private function persist(array $data): bool
     {
+        // Update local cache.
+        $this->data = $data;
+
+        // Persist new data.
         return update_option($this->option_name, $this->data);
     }
 
 
+    //// Sanitization //////////////////////////////////////////////////////////
+
     /**
      * Sanitize $settings array: only keep known keys, provide default values for missing keys.
      *
@@ -228,7 +228,7 @@ public function persist(): bool
     public function sanitize(array $settings, array $defaults = []): array
     {
         // If no default values are provided, use data from internal cache as default values.
-        $values = ($defaults === []) ? $this->data : $defaults;
+        $values = ($defaults === []) ? $this->get() : $defaults;
 
         // Loop over default values instead of provided $settings - this way only known keys are preserved.
         foreach ($values as $key => $default_value) {
@@ -258,7 +258,7 @@ public function sanitize(array $settings, array $defaults = []): array
      *
      * @return mixed[]|bool|float|int|string
      */
-    protected static function sanitizeByType(mixed $value, array|bool|float|int|string $default): array|bool|float|int|string
+    private static function sanitizeByType(mixed $value, array|bool|float|int|string $default): array|bool|float|int|string
     {
         if (\is_bool($default)) {
             return (bool) $value;
@@ -281,43 +281,13 @@ protected static function sanitizeByType(mixed $value, array|bool|float|int|stri
      *
      * @return string[]
      */
-    protected static function parseList(array|string $list): array
+    private static function parseList(array|string $list): array
     {
         return \is_array($list) ? $list : \array_filter(\array_map('trim', \explode(PHP_EOL, $list)));
     }
 
 
-    /**
-     * Update setting under $name with $value. Store update values in DB.
-     *
-     * @param string $name
-     * @param mixed $value
-     *
-     * @return bool
-     */
-    public function update(string $name, mixed $value): bool
-    {
-        if (!isset($this->data[$name])) {
-            // Cannot update, invalid setting name.
-            return false;
-        }
-
-        $data = $this->data;
-
-        if (null === $value) {
-            // Null value unsets (resets) setting to default state
-            unset($data[$name]);
-        } else {
-            // Any other value updates it
-            $data[$name] = $value;
-        }
-
-        // Sanitize new value and update cache.
-        $this->data = $this->sanitize($data);
-        // Make changes permanent.
-        return $this->persist();
-    }
-
+    //// Update hook callback //////////////////////////////////////////////////
 
     /**
      * Execute provided $callback as soon as settings are updated and persisted.

diff --git a/classes/BlueChip/Security/Helpers/Is.php b/classes/BlueChip/Security/Helpers/Is.php
@@ -65,7 +65,7 @@ public static function request(string $type): bool
             case 'frontend':
                 return (!is_admin() || wp_doing_ajax()) && !wp_doing_cron();
             case 'wp-cli':
-                return \defined('WP_CLI') && WP_CLI;
+                return \defined('WP_CLI') && \constant('WP_CLI');
             default:
                 _doing_it_wrong(__METHOD__, \sprintf('Unknown request type: %s', $type), '0.1.0');
                 return false;

diff --git a/classes/BlueChip/Security/Modules/BadRequestsBanner/BuiltInRules.php b/classes/BlueChip/Security/Modules/BadRequestsBanner/BuiltInRules.php
@@ -10,6 +10,10 @@ abstract class BuiltInRules
 
     private const ARCHIVE_FILES_PATTERN = '\.(tgz|zip)$';
 
+    public const ASP_FILES = 'asp-files';
+
+    private const ASP_FILES_PATTERN = '\.aspx?$';
+
     public const BACKUP_FILES = 'backup-files';
 
     private const BACKUP_FILES_PATTERN = 'backup|(\.(back|old|tmp)$)';
@@ -28,6 +32,11 @@ abstract class BuiltInRules
     public static function enlist(): array
     {
         return [
+            self::ASP_FILES => new BanRule(
+                __('Non-existent ASP files', 'bc-security'),
+                self::ASP_FILES_PATTERN,
+                __('(any URI targeting file with .asp or .aspx extension)', 'bc-security')
+            ),
             self::PHP_FILES => new BanRule(
                 __('Non-existent PHP files', 'bc-security'),
                 self::PHP_FILES_PATTERN,