diff --git a/.gitignore b/.gitignore index b0c31dd..24de360 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ composer.lock package-lock.json /lib/ +tests/.phpunit.result.cache \ No newline at end of file diff --git a/Modules/Sfw.php b/Modules/Sfw.php index d5da741..1205141 100644 --- a/Modules/Sfw.php +++ b/Modules/Sfw.php @@ -286,6 +286,8 @@ public function diePage($result) $request_uri = preg_replace('%sfw_test_ip=\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}&?%', '', $request_uri); } + $request_uri = htmlspecialchars($request_uri, ENT_QUOTES | ENT_HTML5, 'UTF-8'); + // @ToDo not implemented yet // Custom Logo //$custom_logo_img = ''; diff --git a/composer.json b/composer.json index 0aa27ca..d8b6669 100644 --- a/composer.json +++ b/composer.json @@ -24,7 +24,7 @@ }, "require-dev": { "vimeo/psalm": "^4.8", - "phpunit/phpunit": "^7.5", + "phpunit/phpunit": "^8.5.52", "squizlabs/php_codesniffer": "3.*", "phpcompatibility/php-compatibility": "^9.3" }, @@ -41,5 +41,10 @@ "psr-4": { "Cleantalk\\Common\\Firewall\\": "/" } + }, + "config": { + "allow-plugins": { + "cleantalk/apbct-installer": true + } } } diff --git a/tests/Modules/SFW/SfwXssProtectionTest.php b/tests/Modules/SFW/SfwXssProtectionTest.php new file mode 100644 index 0000000..4393891 --- /dev/null +++ b/tests/Modules/SFW/SfwXssProtectionTest.php @@ -0,0 +1,174 @@ +assertSame($expectedOutput, $sanitized); + + // Additional check: the result should not contain unescaped tags + $this->assertStringNotContainsString('assertStringContainsString('/', $sanitized); + } + + /** + * Data provider with various XSS payloads + */ + public static function xssPayloadProvider(): array + { + return [ + 'basic script tag' => [ + '/', + '/<script>alert("XSS")</script>' + ], + 'script with single quotes' => [ + "/", + '/<script>alert('XSS')</script>' + ], + 'img onerror' => [ + '/', + '/<img src=x onerror=alert(1)>' + ], + 'svg onload' => [ + '/', + '/<svg onload=alert(1)>' + ], + 'javascript protocol' => [ + '/page?url=javascript:alert(1)', + '/page?url=javascript:alert(1)' + ], + 'encoded angle brackets' => [ + '/page?q=%3Cscript%3Ealert(1)%3C/script%3E', + '/page?q=%3Cscript%3Ealert(1)%3C/script%3E' + ], + 'double quotes in attribute' => [ + '/page?name=">', + '/page?name="><script>alert(1)</script>' + ], + 'single quotes in attribute' => [ + "/page?name='>", + '/page?name='><script>alert(1)</script>' + ], + 'event handler injection' => [ + '/page" onmouseover="alert(1)', + '/page" onmouseover="alert(1)' + ], + 'mixed quotes' => [ + '/page?a="test\'value"', + '/page?a="test'value"' + ], + 'html entities' => [ + '/page?q=bold&test', + '/page?q=<b>bold</b>&amp;test' + ], + ]; + } + + /** + * Data provider with safe URLs + */ + public static function safeUrlProvider(): array + { + return [ + 'simple path' => ['/page'], + 'path with query' => ['/page?id=123'], + 'path with multiple params' => ['/page?id=123&name=test'], + 'nested path' => ['/admin/users/edit'], + 'path with numbers' => ['/article/2024/01/15'], + 'path with hyphens' => ['/my-page-url'], + 'path with underscores' => ['/my_page_url'], + 'root path' => ['/'], + 'path with hash fragment' => ['/page#section'], + 'complex query string' => ['/search?q=test+query&page=1&sort=asc'], + ]; + } + + /** + * Checks that an empty string is handled correctly + */ + public function testEmptyRequestUri() + { + $sanitized = htmlspecialchars('', ENT_QUOTES | ENT_HTML5, 'UTF-8'); + $this->assertSame('', $sanitized); + } + + /** + * Checks that null is converted to an empty string + */ + public function testNullRequestUri() + { + $sanitized = htmlspecialchars((string)null, ENT_QUOTES | ENT_HTML5, 'UTF-8'); + $this->assertSame('', $sanitized); + } + + /** + * Checks that UTF-8 characters are preserved + */ + public function testUtf8Characters() + { + $input = '/page?query=test'; + $sanitized = htmlspecialchars($input, ENT_QUOTES | ENT_HTML5, 'UTF-8'); + + // UTF-8 characters should remain unchanged + $this->assertSame($input, $sanitized); + } + + /** + * Checks that the sfw_test_ip parameter is correctly removed before sanitization + */ + public function testSfwTestIpRemoval() + { + $input = '/page?sfw_test_ip=192.168.1.1&other=value'; + + // Emulate the logic from Sfw::diePage() + $request_uri = preg_replace('%sfw_test_ip=\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}&?%', '', $input); + $sanitized = htmlspecialchars($request_uri, ENT_QUOTES | ENT_HTML5, 'UTF-8'); + + $this->assertStringNotContainsString('sfw_test_ip', $sanitized); + $this->assertStringContainsString('other=value', $sanitized); + } + + /** + * Checks that XSS in sfw_test_ip does not pass even if the parameter is not completely removed + */ + public function testSfwTestIpWithXssPayload() + { + // Attempted injection through an invalid IP + $input = '/page?sfw_test_ip=&other=value'; + + // Regex won't match (invalid IP), so the parameter remains + $request_uri = preg_replace('%sfw_test_ip=\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}&?%', '', $input); + $sanitized = htmlspecialchars($request_uri, ENT_QUOTES | ENT_HTML5, 'UTF-8'); + + // XSS should still be escaped + $this->assertStringNotContainsString('