diff --git a/.env.example b/.env.example index ad267516d..e51bf8067 100644 --- a/.env.example +++ b/.env.example @@ -7,31 +7,20 @@ CI_ENVIRONMENT = production #-------------------------------------------------------------------- # SECURITY: ALLOWED HOSTNAMES #-------------------------------------------------------------------- -# IMPORTANT: Whitelist of allowed hostnames to prevent Host Header +# CRITICAL: Whitelist of allowed hostnames to prevent Host Header # Injection attacks (GHSA-jchf-7hr6-h4f3). # -# If not configured, the application will default to 'localhost', -# which may break functionality in production. +# REQUIRED IN PRODUCTION: Application will fail to start if not configured. +# In development, falls back to 'localhost' with an error log. # -# Configure this with all domains/subdomains that host your application: -# - Primary domain -# - WWW subdomain (if used) -# - Any alternative domains +# Configure with comma-separated list of domains/subdomains: +# app.allowedHostnames = 'yourdomain.com,www.yourdomain.com' # -# Examples: -# Single domain: -# app.allowedHostnames.0 = 'example.com' +# For local development: +# app.allowedHostnames = 'localhost' # -# Multiple domains: -# app.allowedHostnames.0 = 'example.com' -# app.allowedHostnames.1 = 'www.example.com' -# app.allowedHostnames.2 = 'demo.opensourcepos.org' -# -# For localhost development: -# app.allowedHostnames.0 = 'localhost' -# -# Note: Do not include the protocol (http/https) or port number. -#app.allowedHostnames.0 = '' +# Note: Do not include protocol (http/https) or port numbers. +app.allowedHostnames = '' #-------------------------------------------------------------------- # DATABASE diff --git a/INSTALL.md b/INSTALL.md index b63ff893f..32edff423 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -8,26 +8,36 @@ ## Security Configuration -### Allowed Hostnames (Required for Production) +### Allowed Hostnames (REQUIRED for Production) -OpenSourcePOS validates the Host header against a whitelist to prevent Host Header Injection attacks (GHSA-jchf-7hr6-h4f3). **You must configure this for production deployments.** +⚠️ **CRITICAL**: OpenSourcePOS validates the Host header to prevent Host Header Injection attacks (GHSA-jchf-7hr6-h4f3). **You MUST configure `app.allowedHostnames` for production deployments. If not configured, the application will fail to start.** -Add the following to your `.env` file: +**Add to your `.env` file:** -``` -app.allowedHostnames.0 = 'yourdomain.com' -app.allowedHostnames.1 = 'www.yourdomain.com' +```bash +# Comma-separated list of allowed hostnames (no protocols or ports) +app.allowedHostnames = 'yourdomain.com,www.yourdomain.com' ``` -**For local development**, use: -``` -app.allowedHostnames.0 = 'localhost' +**For local development:** + +```bash +app.allowedHostnames = 'localhost' ``` -If `allowedHostnames` is not configured: -1. A security warning will be logged -2. The application will fall back to 'localhost' as the hostname -3. This means URLs generated by the application (links, redirects, etc.) will point to 'localhost' +**If you see this error at startup:** + +```text +RuntimeException: Security: allowedHostnames is not configured. +``` + +**Solution**: Add `app.allowedHostnames` to your `.env` file with your domain(s). + +**Why this matters:** +- Prevents Host Header Injection attacks (GHSA-jchf-7hr6-h4f3) +- Ensures URLs are generated with the correct domain +- Security advisory: https://github.com/opensourcepos/opensourcepos/security/advisories/GHSA-jchf-7hr6-h4f3 +- Fixes issue #4480: .env configuration now works via comma-separated values ### HTTPS Behind Proxy diff --git a/app/Config/App.php b/app/Config/App.php index a24c70dd0..6ecff69eb 100644 --- a/app/Config/App.php +++ b/app/Config/App.php @@ -62,14 +62,14 @@ class App extends BaseConfig * an entry in this list, the request will use the first allowed hostname. * * IMPORTANT: This MUST be configured for production deployments. - * If empty, the application will fall back to 'localhost'. + * If empty in production, the application will fail to start. + * In development, it will fall back to 'localhost' with a warning. * - * Configure via .env file: - * app.allowedHostnames.0 = 'example.com' - * app.allowedHostnames.1 = 'www.example.com' + * Configure via .env file (comma-separated list): + * app.allowedHostnames = 'example.com,www.example.com' * * For local development: - * app.allowedHostnames.0 = 'localhost' + * app.allowedHostnames = 'localhost' * * @var list */ @@ -291,6 +291,17 @@ class App extends BaseConfig public function __construct() { parent::__construct(); + + // Solution for CodeIgniter 4 limitation: arrays cannot be set from .env + // See: https://github.com/codeigniter4/CodeIgniter4/issues/7311 + $envAllowedHostnames = getenv('app.allowedHostnames'); + if ($envAllowedHostnames !== false && trim($envAllowedHostnames) !== '') { + $this->allowedHostnames = array_values(array_filter( + array_map('trim', explode(',', $envAllowedHostnames)), + static fn (string $hostname): bool => $hostname !== '' + )); + } + $this->https_on = (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on') || (isset($_ENV['FORCE_HTTPS']) && $_ENV['FORCE_HTTPS'] == 'true'); $host = $this->getValidHost(); @@ -305,19 +316,36 @@ class App extends BaseConfig * Security: Prevents Host Header Injection attacks (GHSA-jchf-7hr6-h4f3) * by validating the HTTP_HOST against a whitelist of allowed hostnames. * + * In production: Fails fast if allowedHostnames is not configured. + * In development: Allows localhost fallback with an error log. + * * @return string A validated hostname + * @throws \RuntimeException If allowedHostnames is not configured in production */ private function getValidHost(): string { $httpHost = $_SERVER['HTTP_HOST'] ?? 'localhost'; + + // Determine environment + // CodeIgniter's test bootstrap sets $_SERVER['CI_ENVIRONMENT'] = 'testing' + // Check $_SERVER first, then $_ENV, then fall back to 'production' + $environment = $_SERVER['CI_ENVIRONMENT'] ?? $_ENV['CI_ENVIRONMENT'] ?? getenv('CI_ENVIRONMENT') ?: 'production'; if (empty($this->allowedHostnames)) { - log_message('warning', + $errorMessage = 'Security: allowedHostnames is not configured. ' . 'Host header injection protection is disabled. ' . - 'Please set app.allowedHostnames in your .env file. ' . - 'Received Host: ' . $httpHost - ); + 'Set app.allowedHostnames in your .env file. ' . + 'Example: app.allowedHostnames = "example.com,www.example.com" ' . + 'Received Host: ' . $httpHost; + + // Production: Fail explicitly to prevent silent security vulnerabilities + // Testing and development: Allow localhost fallback + if ($environment === 'production') { + throw new \RuntimeException($errorMessage); + } + + log_message('error', $errorMessage . ' Using localhost fallback (development only).'); return 'localhost'; } @@ -325,6 +353,7 @@ class App extends BaseConfig return $httpHost; } + // Host not in whitelist - use first configured hostname as fallback log_message('warning', 'Security: Rejected HTTP_HOST "' . $httpHost . '" - not in allowedHostnames whitelist. ' . 'Using fallback: ' . $this->allowedHostnames[0] diff --git a/tests/Config/AppTest.php b/tests/Config/AppTest.php index 04701c24f..601378ca7 100644 --- a/tests/Config/AppTest.php +++ b/tests/Config/AppTest.php @@ -15,6 +15,10 @@ class AppTest extends CIUnitTestCase protected function tearDown(): void { parent::tearDown(); + // Clean up environment + putenv('CI_ENVIRONMENT'); + putenv('app.allowedHostnames'); + unset($_SERVER['HTTP_HOST']); } public function testGetValidHostReturnsHostWhenValid(): void @@ -59,8 +63,11 @@ class AppTest extends CIUnitTestCase $this->assertEquals('example.com', $host); } - public function testGetValidHostReturnsLocalhostWhenNoWhitelist(): void + public function testGetValidHostReturnsLocalhostInDevelopmentWhenNoWhitelist(): void { + // Set development environment + putenv('CI_ENVIRONMENT=development'); + $app = new class extends App { public array $allowedHostnames = []; @@ -80,6 +87,28 @@ class AppTest extends CIUnitTestCase $this->assertEquals('localhost', $host); } + public function testGetValidHostThrowsExceptionInProductionWhenNoWhitelist(): void + { + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('allowedHostnames is not configured'); + + // Set production environment + putenv('CI_ENVIRONMENT=production'); + + $app = new class extends App { + public array $allowedHostnames = []; + + public function __construct() {} + }; + + $reflection = new \ReflectionClass($app); + $method = $reflection->getMethod('getValidHost'); + $method->setAccessible(true); + + $_SERVER['HTTP_HOST'] = 'malicious.com'; + $method->invoke($app); + } + public function testGetValidHostHandlesMissingHttpHost(): void { $app = new class extends App { @@ -123,4 +152,133 @@ class AppTest extends CIUnitTestCase $this->assertStringContainsString('example.com', $app->baseURL); $this->assertStringNotContainsString('malicious.com', $app->baseURL); } + + public function testEnvAllowedHostnamesParsedAsCommaSeparated(): void + { + // Set environment variable + putenv('app.allowedHostnames=example.com,www.example.com,demo.example.com'); + + $_SERVER['HTTP_HOST'] = 'www.example.com'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + $_SERVER['HTTPS'] = null; + + $app = new App(); + + // Constructor should parse comma-separated values + $this->assertEquals(['example.com', 'www.example.com', 'demo.example.com'], $app->allowedHostnames); + $this->assertStringContainsString('www.example.com', $app->baseURL); + + // Clean up + putenv('app.allowedHostnames'); + } + + public function testEnvAllowedHostnamesTrimmedWhitespace(): void + { + // Set environment variable with whitespace + putenv('app.allowedHostnames= example.com , www.example.com , demo.example.com '); + + $_SERVER['HTTP_HOST'] = 'example.com'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + $_SERVER['HTTPS'] = null; + + $app = new App(); + + // Values should be trimmed + $this->assertEquals(['example.com', 'www.example.com', 'demo.example.com'], $app->allowedHostnames); + + // Clean up + putenv('app.allowedHostnames'); + } + + public function testEnvAllowedHostnamesSingleValue(): void + { + // Set environment variable with single value + putenv('app.allowedHostnames=localhost'); + + $_SERVER['HTTP_HOST'] = 'localhost'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + $_SERVER['HTTPS'] = null; + + $app = new App(); + + // Single value should work + $this->assertEquals(['localhost'], $app->allowedHostnames); + $this->assertStringContainsString('localhost', $app->baseURL); + + // Clean up + putenv('app.allowedHostnames'); + } + + public function testEnvAllowedHostnamesEmptyStringNotConfigured(): void + { + // Set environment variable to empty string + putenv('app.allowedHostnames='); + + // Set development environment + putenv('CI_ENVIRONMENT=development'); + + $_SERVER['HTTP_HOST'] = 'example.com'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + $_SERVER['HTTPS'] = null; + + $app = new App(); + + // Empty string should be treated as not configured + $this->assertEquals([], $app->allowedHostnames); + + // In development, should fall back to localhost + $this->assertStringContainsString('localhost', $app->baseURL); + + // Clean up + putenv('app.allowedHostnames'); + putenv('CI_ENVIRONMENT'); + } + + public function testEnvAllowedHostnamesFiltersEmptyEntries(): void + { + // Trailing comma should not produce empty entry + putenv('app.allowedHostnames=example.com,'); + $_SERVER['HTTP_HOST'] = 'example.com'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + $_SERVER['HTTPS'] = null; + + $app = new App(); + $this->assertEquals(['example.com'], $app->allowedHostnames); + + // Clean up + putenv('app.allowedHostnames'); + + // Leading comma should not produce empty entry + putenv('app.allowedHostnames=,example.com'); + $_SERVER['HTTP_HOST'] = 'example.com'; + + $app = new App(); + $this->assertEquals(['example.com'], $app->allowedHostnames); + + // Clean up + putenv('app.allowedHostnames'); + + // Whitespace-only entry should be filtered + putenv('app.allowedHostnames=example.com, ,www.example.com'); + $_SERVER['HTTP_HOST'] = 'example.com'; + + $app = new App(); + $this->assertEquals(['example.com', 'www.example.com'], $app->allowedHostnames); + + // Clean up + putenv('app.allowedHostnames'); + + // All-whitespace value should be treated as not configured + putenv('CI_ENVIRONMENT=development'); + putenv('app.allowedHostnames= , , '); + $_SERVER['HTTP_HOST'] = 'example.com'; + + $app = new App(); + $this->assertEquals([], $app->allowedHostnames); + $this->assertStringContainsString('localhost', $app->baseURL); + + // Clean up + putenv('app.allowedHostnames'); + putenv('CI_ENVIRONMENT'); + } } \ No newline at end of file