From f8cfbf59fdbafde6d50962ccd8002a1a4e72e779 Mon Sep 17 00:00:00 2001 From: Amin Date: Fri, 23 Feb 2024 12:44:05 +0100 Subject: [PATCH 1/3] Add raw string filter for environment variables This allows secret-like environment variables such as db passwords to contain special characters and not being escaped by previous `FILTER_STRING` filter. --- src/Env/Filters.php | 16 ++++++++++++ src/Env/WordPressEnvBridge.php | 26 +++++++++---------- tests/fixtures/example.env | 2 +- .../Env/WordPressEnvBridgeTest.php | 2 +- tests/unit/Env/FiltersTest.php | 13 ++++++++++ 5 files changed, 44 insertions(+), 15 deletions(-) diff --git a/src/Env/Filters.php b/src/Env/Filters.php index d5d276e..a087eb4 100644 --- a/src/Env/Filters.php +++ b/src/Env/Filters.php @@ -33,6 +33,7 @@ final class Filters public const FILTER_INT_OR_BOOL = 'int|bool'; public const FILTER_STRING_OR_BOOL = 'string|bool'; public const FILTER_STRING = 'string'; + public const FILTER_RAW_STRING = 'raw-string'; public const FILTER_OCTAL_MOD = 'mod'; public const FILTER_TABLE_PREFIX = 'table-prefix'; @@ -87,6 +88,8 @@ private function applyFilter(string $mode, $value) return $this->filterFloat($value); case self::FILTER_STRING: return $this->filterString($value); + case self::FILTER_RAW_STRING: + return $this->filterRawString($value); case self::FILTER_INT_OR_BOOL: return $this->filterIntOrBool($value); case self::FILTER_STRING_OR_BOOL: @@ -157,6 +160,19 @@ private function filterString($value): string return htmlspecialchars(strip_tags((string)$value), ENT_QUOTES, 'UTF-8', false); } + /** + * @param mixed $value + * @return string + */ + private function filterRawString($value): string + { + if (!is_scalar($value)) { + throw new \Exception('Invalid scalar.'); + } + + return addslashes((string)$value); + } + /** * @param mixed $value * @return bool|int diff --git a/src/Env/WordPressEnvBridge.php b/src/Env/WordPressEnvBridge.php index a670f51..5576d76 100644 --- a/src/Env/WordPressEnvBridge.php +++ b/src/Env/WordPressEnvBridge.php @@ -25,8 +25,8 @@ class WordPressEnvBridge 'ALLOW_UNFILTERED_UPLOADS' => Filters::FILTER_BOOL, 'ALTERNATE_WP_CRON' => Filters::FILTER_BOOL, 'AUTH_COOKIE' => Filters::FILTER_STRING, - 'AUTH_KEY' => Filters::FILTER_STRING, - 'AUTH_SALT' => Filters::FILTER_STRING, + 'AUTH_KEY' => Filters::FILTER_RAW_STRING, + 'AUTH_SALT' => Filters::FILTER_RAW_STRING, 'AUTOMATIC_UPDATER_DISABLED' => Filters::FILTER_BOOL, 'AUTOSAVE_INTERVAL' => Filters::FILTER_INT, @@ -50,7 +50,7 @@ class WordPressEnvBridge 'DB_COLLATE' => Filters::FILTER_STRING, 'DB_HOST' => Filters::FILTER_STRING, 'DB_NAME' => Filters::FILTER_STRING, - 'DB_PASSWORD' => Filters::FILTER_STRING, + 'DB_PASSWORD' => Filters::FILTER_RAW_STRING, 'DB_USER' => Filters::FILTER_STRING, 'DIEONDBERROR' => Filters::FILTER_BOOL, 'DISABLE_WP_CRON' => Filters::FILTER_BOOL, @@ -80,7 +80,7 @@ class WordPressEnvBridge 'FTP_FORCE' => Filters::FILTER_BOOL, 'FTP_HOST' => Filters::FILTER_STRING, 'FTP_LANG_DIR' => Filters::FILTER_STRING, - 'FTP_PASS' => Filters::FILTER_STRING, + 'FTP_PASS' => Filters::FILTER_RAW_STRING, 'FTP_PLUGIN_DIR' => Filters::FILTER_STRING, 'FTP_PRIKEY' => Filters::FILTER_STRING, 'FTP_PUBKEY' => Filters::FILTER_STRING, @@ -97,8 +97,8 @@ class WordPressEnvBridge 'LANGDIR' => Filters::FILTER_STRING, 'LOGGED_IN_COOKIE' => Filters::FILTER_STRING, - 'LOGGED_IN_KEY' => Filters::FILTER_STRING, - 'LOGGED_IN_SALT' => Filters::FILTER_STRING, + 'LOGGED_IN_KEY' => Filters::FILTER_RAW_STRING, + 'LOGGED_IN_SALT' => Filters::FILTER_RAW_STRING, 'MEDIA_TRASH' => Filters::FILTER_BOOL, 'MULTISITE' => Filters::FILTER_BOOL, @@ -108,8 +108,8 @@ class WordPressEnvBridge 'MYSQL_NEW_LINK' => Filters::FILTER_BOOL, 'NOBLOGREDIRECT' => Filters::FILTER_STRING, - 'NONCE_KEY' => Filters::FILTER_STRING, - 'NONCE_SALT' => Filters::FILTER_STRING, + 'NONCE_KEY' => Filters::FILTER_RAW_STRING, + 'NONCE_SALT' => Filters::FILTER_RAW_STRING, 'NO_HEADER_TEXT' => Filters::FILTER_STRING, 'PASS_COOKIE' => Filters::FILTER_STRING, @@ -129,11 +129,11 @@ class WordPressEnvBridge 'SAVEQUERIES' => Filters::FILTER_BOOL, 'SCRIPT_DEBUG' => Filters::FILTER_BOOL, - 'SECRET_KEY' => Filters::FILTER_STRING, - 'SECRET_SALT' => Filters::FILTER_STRING, + 'SECRET_KEY' => Filters::FILTER_RAW_STRING, + 'SECRET_SALT' => Filters::FILTER_RAW_STRING, 'SECURE_AUTH_COOKIE' => Filters::FILTER_STRING, - 'SECURE_AUTH_KEY' => Filters::FILTER_STRING, - 'SECURE_AUTH_SALT' => Filters::FILTER_STRING, + 'SECURE_AUTH_KEY' => Filters::FILTER_RAW_STRING, + 'SECURE_AUTH_SALT' => Filters::FILTER_RAW_STRING, 'SHORTINIT' => Filters::FILTER_BOOL, 'SITECOOKIEPATH' => Filters::FILTER_STRING, 'SITE_ID_CURRENT_SITE' => Filters::FILTER_INT, @@ -177,7 +177,7 @@ class WordPressEnvBridge 'WP_POST_REVISIONS' => Filters::FILTER_INT_OR_BOOL, 'WP_PROXY_BYPASS_HOSTS' => Filters::FILTER_STRING, 'WP_PROXY_HOST' => Filters::FILTER_STRING, - 'WP_PROXY_PASSWORD' => Filters::FILTER_STRING, + 'WP_PROXY_PASSWORD' => Filters::FILTER_RAW_STRING, 'WP_PROXY_PORT' => Filters::FILTER_INT, 'WP_PROXY_USERNAME' => Filters::FILTER_STRING, 'WP_SITEURL' => Filters::FILTER_STRING, diff --git a/tests/fixtures/example.env b/tests/fixtures/example.env index 33351bd..10bfa22 100644 --- a/tests/fixtures/example.env +++ b/tests/fixtures/example.env @@ -1,6 +1,6 @@ DB_HOST=localhost DB_NAME=wp -DB_PASSWORD="my secret!" +DB_PASSWORD="foo&bar!bazread('DB_HOST')); static::assertSame('wp', $bridge->read('DB_NAME')); - static::assertSame('my secret!', $bridge->read('DB_PASSWORD')); + static::assertSame('foo&bar!bazread('DB_PASSWORD')); static::assertSame('xxx_', $bridge->read('DB_TABLE_PREFIX')); static::assertSame('wp_user', $bridge->read('DB_USER')); static::assertSame('', $bridge->read('COOKIE_DOMAIN')); diff --git a/tests/unit/Env/FiltersTest.php b/tests/unit/Env/FiltersTest.php index e94247d..c8e9452 100644 --- a/tests/unit/Env/FiltersTest.php +++ b/tests/unit/Env/FiltersTest.php @@ -84,6 +84,19 @@ public static function filterDataProvider(): array [Filters::FILTER_STRING, [], null], [Filters::FILTER_STRING, '', ''], [Filters::FILTER_STRING, "", ''], + [Filters::FILTER_RAW_STRING, 1, '1'], + [Filters::FILTER_RAW_STRING, 123.456, '123.456'], + [Filters::FILTER_RAW_STRING, 0, '0'], + [Filters::FILTER_RAW_STRING, new \ArrayObject(), null], + [Filters::FILTER_RAW_STRING, false, ''], + [Filters::FILTER_RAW_STRING, true, '1'], + [Filters::FILTER_RAW_STRING, [], null], + [Filters::FILTER_RAW_STRING, '', ''], + [Filters::FILTER_RAW_STRING, "", ''], + [Filters::FILTER_RAW_STRING, 'hello!', 'hello!'], + [Filters::FILTER_RAW_STRING, 'foo&bar', 'foo&bar'], + [Filters::FILTER_RAW_STRING, 'foo Date: Fri, 23 Feb 2024 13:10:52 +0100 Subject: [PATCH 2/3] Ensure consistency between constant and env variable in generated cache --- src/Env/WordPressEnvBridge.php | 17 ++++++++++------- tests/fixtures/example.env | 1 + .../integration/Env/WordPressEnvBridgeTest.php | 7 +++++++ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/Env/WordPressEnvBridge.php b/src/Env/WordPressEnvBridge.php index 5576d76..dd0b9c5 100644 --- a/src/Env/WordPressEnvBridge.php +++ b/src/Env/WordPressEnvBridge.php @@ -565,15 +565,17 @@ public function dumpCached(string $file): bool $symfonyLoaded and $content .= "putenv('SYMFONY_DOTENV_VARS={$symfonyLoaded}');\n\n"; foreach (self::$cache as $key => list($value, $filtered)) { - $slashed = str_replace("'", "\'", $value); + $slashed = addslashes($value); + $preparedValue = $value !== $filtered + ? $filtered + : $slashed; + // For defined constants, dump the `define` with filtered value, if any. if ( in_array($key, $this->definedConstants, true) || array_key_exists($key, self::WP_CONSTANTS) ) { - $define = $value !== $filtered - ? var_export($filtered, true) // phpcs:ignore - : "'{$slashed}'"; + $define = var_export($preparedValue, true); // phpcs:ignore $content .= "define('{$key}', {$define});\n"; } @@ -584,9 +586,10 @@ public function dumpCached(string $file): bool } // For env loaded from file, dump the variable definition. - $content .= "putenv('{$key}={$slashed}');\n"; - $content .= "\$_ENV['{$key}'] = '{$slashed}';\n"; - (strpos($key, 'HTTP_') !== 0) and $content .= "\$_SERVER['{$key}'] = '{$slashed}';\n\n"; + $content .= "putenv('{$key}={$preparedValue}');\n"; + $content .= "\$_ENV['{$key}'] = '{$preparedValue}';\n"; + (strpos($key, 'HTTP_') !== 0) + and $content .= "\$_SERVER['{$key}'] = '{$preparedValue}';\n\n"; } $content .= sprintf("return %s;\n", var_export(static::$cache, true)); // phpcs:ignore diff --git a/tests/fixtures/example.env b/tests/fixtures/example.env index 10bfa22..251940a 100644 --- a/tests/fixtures/example.env +++ b/tests/fixtures/example.env @@ -21,3 +21,4 @@ PLUGIN_CONFIG_TWO=2 PLUGIN_CONFIG_THREE=true PLUGIN_CONFIG_FOUR=4 PLUGIN_CONFIG_FIVE=5 +AUTH_COOKIE=BAD&Authread('DB_TABLE_PREFIX')); static::assertSame('wp_user', $bridge->read('DB_USER')); static::assertSame('', $bridge->read('COOKIE_DOMAIN')); + static::assertSame('BAD&Auth', $bridge->read('AUTH_COOKIE')); } /** @@ -417,6 +418,12 @@ function (): array { static::assertSame('localhost', $cachedBridge->read('DB_HOST')); // ...and it should still be able to read things from actual env set after cache was built static::assertSame('XYZ', $cachedBridge->read('XYZ')); + + // Test consitency of defined constansts, getenv, $_ENV and $_SERVER in filtered variables + static::assertSame('BAD&Auth', AUTH_COOKIE); + static::assertSame('BAD&Auth', getenv('AUTH_COOKIE')); + static::assertSame('BAD&Auth', $_ENV['AUTH_COOKIE'] ?? null); + static::assertSame('BAD&Auth', $_SERVER['AUTH_COOKIE'] ?? null); } /** From 78846d2c186d4413a7338f8f161c1cd86adce791 Mon Sep 17 00:00:00 2001 From: Amin Date: Tue, 12 Mar 2024 13:27:19 +0100 Subject: [PATCH 3/3] Revert "Ensure consistency between constant and env variable in generated cache" This reverts commit dc86bff7f760457b4aa7f6d536452745cab5df12. --- src/Env/WordPressEnvBridge.php | 17 +++++++---------- tests/fixtures/example.env | 1 - .../integration/Env/WordPressEnvBridgeTest.php | 7 ------- 3 files changed, 7 insertions(+), 18 deletions(-) diff --git a/src/Env/WordPressEnvBridge.php b/src/Env/WordPressEnvBridge.php index dd0b9c5..5576d76 100644 --- a/src/Env/WordPressEnvBridge.php +++ b/src/Env/WordPressEnvBridge.php @@ -565,17 +565,15 @@ public function dumpCached(string $file): bool $symfonyLoaded and $content .= "putenv('SYMFONY_DOTENV_VARS={$symfonyLoaded}');\n\n"; foreach (self::$cache as $key => list($value, $filtered)) { - $slashed = addslashes($value); - $preparedValue = $value !== $filtered - ? $filtered - : $slashed; - + $slashed = str_replace("'", "\'", $value); // For defined constants, dump the `define` with filtered value, if any. if ( in_array($key, $this->definedConstants, true) || array_key_exists($key, self::WP_CONSTANTS) ) { - $define = var_export($preparedValue, true); // phpcs:ignore + $define = $value !== $filtered + ? var_export($filtered, true) // phpcs:ignore + : "'{$slashed}'"; $content .= "define('{$key}', {$define});\n"; } @@ -586,10 +584,9 @@ public function dumpCached(string $file): bool } // For env loaded from file, dump the variable definition. - $content .= "putenv('{$key}={$preparedValue}');\n"; - $content .= "\$_ENV['{$key}'] = '{$preparedValue}';\n"; - (strpos($key, 'HTTP_') !== 0) - and $content .= "\$_SERVER['{$key}'] = '{$preparedValue}';\n\n"; + $content .= "putenv('{$key}={$slashed}');\n"; + $content .= "\$_ENV['{$key}'] = '{$slashed}';\n"; + (strpos($key, 'HTTP_') !== 0) and $content .= "\$_SERVER['{$key}'] = '{$slashed}';\n\n"; } $content .= sprintf("return %s;\n", var_export(static::$cache, true)); // phpcs:ignore diff --git a/tests/fixtures/example.env b/tests/fixtures/example.env index 251940a..10bfa22 100644 --- a/tests/fixtures/example.env +++ b/tests/fixtures/example.env @@ -21,4 +21,3 @@ PLUGIN_CONFIG_TWO=2 PLUGIN_CONFIG_THREE=true PLUGIN_CONFIG_FOUR=4 PLUGIN_CONFIG_FIVE=5 -AUTH_COOKIE=BAD&Authread('DB_TABLE_PREFIX')); static::assertSame('wp_user', $bridge->read('DB_USER')); static::assertSame('', $bridge->read('COOKIE_DOMAIN')); - static::assertSame('BAD&Auth', $bridge->read('AUTH_COOKIE')); } /** @@ -418,12 +417,6 @@ function (): array { static::assertSame('localhost', $cachedBridge->read('DB_HOST')); // ...and it should still be able to read things from actual env set after cache was built static::assertSame('XYZ', $cachedBridge->read('XYZ')); - - // Test consitency of defined constansts, getenv, $_ENV and $_SERVER in filtered variables - static::assertSame('BAD&Auth', AUTH_COOKIE); - static::assertSame('BAD&Auth', getenv('AUTH_COOKIE')); - static::assertSame('BAD&Auth', $_ENV['AUTH_COOKIE'] ?? null); - static::assertSame('BAD&Auth', $_SERVER['AUTH_COOKIE'] ?? null); } /**