From 0fdb8b2041b3dea91ee43f0e2e69304603787c94 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Wed, 29 Apr 2026 17:22:24 +0200 Subject: [PATCH] Validate user passwords in DB managers Also, make the validateParameter method ignore null parameters, e.g. for cases when tenants are created using Tenant::make() without tenancy_db_username set -- $databaseConfig->getUsername() allows null, same should go for the validate method whose only concern is checking strings for invalid characters. --- .../Concerns/ManagesPostgresUsers.php | 2 +- .../Concerns/ValidatesSqlParameters.php | 44 +++++++++++++++++-- ...olledMicrosoftSQLServerDatabaseManager.php | 2 +- ...rmissionControlledMySQLDatabaseManager.php | 2 +- 4 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/Database/Concerns/ManagesPostgresUsers.php b/src/Database/Concerns/ManagesPostgresUsers.php index 09632f9a..ddc03a34 100644 --- a/src/Database/Concerns/ManagesPostgresUsers.php +++ b/src/Database/Concerns/ManagesPostgresUsers.php @@ -28,8 +28,8 @@ trait ManagesPostgresUsers $username = $databaseConfig->getUsername(); $password = $databaseConfig->getPassword(); - // todo@validation password $this->validateParameter($username); + $this->validatePassword($password); $createUser = ! $this->userExists($username); diff --git a/src/Database/Concerns/ValidatesSqlParameters.php b/src/Database/Concerns/ValidatesSqlParameters.php index 970a0e71..ad61e8f4 100644 --- a/src/Database/Concerns/ValidatesSqlParameters.php +++ b/src/Database/Concerns/ValidatesSqlParameters.php @@ -18,22 +18,58 @@ trait ValidatesSqlParameters return 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-'; } + /** + * Characters allowed in database user passwords. + * + * Passwords are always quoted in the SQL statements, so it's safe + * to allow a wider range of characters, as long as it doesn't include + * characters that can break out of the quoted SQL strings (so e.g. + * ', ", \, and ` aren't allowed). + */ + protected static function passwordAllowlist(): string + { + return ' !#$%&()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_abcdefghijklmnopqrstuvwxyz{|}~'; + } + /** * Validate that parameters (database names, usernames, etc.) - * contain only allowed characters before used in SQL statements. + * only contain allowed characters before used in SQL statements. + * + * By default, only the characters in static::parameterAllowlist() are allowed. * * @throws InvalidArgumentException */ - protected function validateParameter(string|array $parameters): string|array + protected function validateParameter(string|array|null $parameters, string|null $allowlist = null): string|array|null { + if (is_null($parameters)) { + // Return null if there's nothing to validate + // (e.g. when $databaseConfig->getUsername() of an + // improperly created tenant is passed). + return null; + } + + $allowlist = $allowlist ?? static::parameterAllowlist(); + foreach ((array) $parameters as $parameter) { foreach (str_split($parameter) as $char) { - if (! str_contains(static::parameterAllowlist(), $char)) { - throw new InvalidArgumentException("Invalid character '{$char}' in SQL parameter: {$parameter}"); + if (! str_contains($allowlist, $char)) { + throw new InvalidArgumentException("Invalid character '{$char}' in parameter: {$parameter}"); } } } return $parameters; } + + /** + * Validate that a password only contains allowed characters before used in SQL statements. + * + * Used as a shorthand for validateParameter() with the less strict allowlist. + * + * @throws InvalidArgumentException + */ + protected function validatePassword(string|null $password): string|null + { + return $this->validateParameter($password, static::passwordAllowlist()); + } } diff --git a/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php b/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php index ee37047b..2671fcb5 100644 --- a/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php @@ -24,8 +24,8 @@ class PermissionControlledMicrosoftSQLServerDatabaseManager extends MicrosoftSQL $username = $databaseConfig->getUsername(); $password = $databaseConfig->getPassword(); - // todo@validation password $this->validateParameter([$database, $username]); + $this->validatePassword($password); // Create login $this->connection()->statement("CREATE LOGIN [$username] WITH PASSWORD = '$password'"); diff --git a/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php b/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php index ad4d6583..d8eb3734 100644 --- a/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php @@ -25,8 +25,8 @@ class PermissionControlledMySQLDatabaseManager extends MySQLDatabaseManager impl $username = $databaseConfig->getUsername(); $password = $databaseConfig->getPassword(); - //todo@validation password $this->validateParameter([$database, $username]); + $this->validatePassword($password); $this->connection()->statement("CREATE USER `{$username}`@`%` IDENTIFIED BY '{$password}'");