fix(settings): update X-XSS-Protection recommendationenh/xss-protection-check

While `1; mode=block` was seen as the most secure value for this header, some years ago, after possible side-channel attacks have become known, this turned towards `0` being the best-practice value, disabling XSS filtering entirely for those old browsers who still support it.

MDN web docs give some explanation: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/X-XSS-Protection#security_considerations

The OWASP cheat sheet recommends `0`: https://cheatsheetseries.owasp.org/cheatsheets/HTTP_Headers_Cheat_Sheet.html#x-xss-protection

Here the related discussion when this recommendation was updated: https://github.com/OWASP/CheatSheetSeries/issues/376

A Stack Overflow question underlines the clear recommendation, adding some more details: https://stackoverflow.com/questions/9090577

Since `1; mode=block` is not a large security risk, it affects only very old browsers, and a changed recommendation will (sadly) trigger a lot of issues/topics in forum and GitHub, the old value however is allowed to pass the check. So to pass the check, either page blocking needs to be enabled along with XSS filtering, or XSS filtering needs to be disabled. The warning however will always suggest to disable it.

Signed-off-by: MichaIng <micha@dietpi.com>

2 files changed, 8 insertions, 8 deletions

diff --git a/apps/settings/lib/SetupChecks/SecurityHeaders.php b/apps/settings/lib/SetupChecks/SecurityHeaders.php
index ed4e56218da..62e2d7535ff 100644
--- a/apps/settings/lib/SetupChecks/SecurityHeaders.php
+++ b/apps/settings/lib/SetupChecks/SecurityHeaders.php
@@ -73,8 +73,8 @@ class SecurityHeaders implements ISetupCheck {
 				}
 
 				$xssFields = array_map('trim', explode(';', $response->getHeader('X-XSS-Protection')));
-				if (!in_array('1', $xssFields) || !in_array('mode=block', $xssFields)) {
-					$msg .= $this->l10n->t('- The `%1$s` HTTP header does not contain `%2$s`. This is a potential security or privacy risk, as it is recommended to adjust this setting accordingly.', ['X-XSS-Protection', '1; mode=block']) . "\n";
+				if (!in_array('1', $xssFields) || !in_array('mode=block', $xssFields) and !in_array('0', $xssFields)) {
+					$msg .= $this->l10n->t('- The `%1$s` HTTP header does not contain `%2$s`. This is a potential security or privacy risk, as it is recommended to adjust this setting accordingly.', ['X-XSS-Protection', '0']) . "\n";
 				}
 
 				$referrerPolicy = $response->getHeader('Referrer-Policy');
diff --git a/apps/settings/tests/SetupChecks/SecurityHeadersTest.php b/apps/settings/tests/SetupChecks/SecurityHeadersTest.php
index e7d87775485..83a72f262c5 100644
--- a/apps/settings/tests/SetupChecks/SecurityHeadersTest.php
+++ b/apps/settings/tests/SetupChecks/SecurityHeadersTest.php
@@ -84,7 +84,7 @@ class SecurityHeadersTest extends TestCase {
 
 		$result = $this->setupcheck->run();
 		$this->assertEquals(
-			"Some headers are not set correctly on your instance\n- The `X-Content-Type-Options` HTTP header is not set to `nosniff`. This is a potential security or privacy risk, as it is recommended to adjust this setting accordingly.\n- The `X-XSS-Protection` HTTP header does not contain `1; mode=block`. This is a potential security or privacy risk, as it is recommended to adjust this setting accordingly.\n",
+			"Some headers are not set correctly on your instance\n- The `X-Content-Type-Options` HTTP header is not set to `nosniff`. This is a potential security or privacy risk, as it is recommended to adjust this setting accordingly.\n- The `X-XSS-Protection` HTTP header does not contain `0`. This is a potential security or privacy risk, as it is recommended to adjust this setting accordingly.\n",
 			$result->getDescription()
 		);
 		$this->assertEquals(SetupResult::WARNING, $result->getSeverity());
@@ -94,7 +94,7 @@ class SecurityHeadersTest extends TestCase {
 		return [
 			// description => modifiedHeaders
 			'basic' => [[]],
-			'extra-xss-protection' => [['X-XSS-Protection' => '1; mode=block; report=https://example.com']],
+			'extra-xss-protection' => [['X-XSS-Protection' => '0; report=https://example.com']],
 			'no-space-in-x-robots' => [['X-Robots-Tag' => 'noindex,nofollow']],
 			'strict-origin-when-cross-origin' => [['Referrer-Policy' => 'strict-origin-when-cross-origin']],
 			'referrer-no-referrer-when-downgrade' => [['Referrer-Policy' => 'no-referrer-when-downgrade']],
@@ -113,7 +113,7 @@ class SecurityHeadersTest extends TestCase {
 	public function testSuccess(array $headers): void {
 		$headers = array_merge(
 			[
-				'X-XSS-Protection' => '1; mode=block',
+				'X-XSS-Protection' => '0',
 				'X-Content-Type-Options' => 'nosniff',
 				'X-Robots-Tag' => 'noindex, nofollow',
 				'X-Frame-Options' => 'SAMEORIGIN',
@@ -140,8 +140,8 @@ class SecurityHeadersTest extends TestCase {
 		return [
 			// description => modifiedHeaders
 			'x-robots-none' => [['X-Robots-Tag' => 'none'], "- The `X-Robots-Tag` HTTP header is not set to `noindex,nofollow`. This is a potential security or privacy risk, as it is recommended to adjust this setting accordingly.\n"],
-			'xss-protection-1' => [['X-XSS-Protection' => '1'], "- The `X-XSS-Protection` HTTP header does not contain `1; mode=block`. This is a potential security or privacy risk, as it is recommended to adjust this setting accordingly.\n"],
-			'xss-protection-0' => [['X-XSS-Protection' => '0'], "- The `X-XSS-Protection` HTTP header does not contain `1; mode=block`. This is a potential security or privacy risk, as it is recommended to adjust this setting accordingly.\n"],
+			'xss-protection-1' => [['X-XSS-Protection' => '1'], "- The `X-XSS-Protection` HTTP header does not contain `0`. This is a potential security or privacy risk, as it is recommended to adjust this setting accordingly.\n"],
+			'xss-protection-0' => [['X-XSS-Protection' => 'mode=block'], "- The `X-XSS-Protection` HTTP header does not contain `0`. This is a potential security or privacy risk, as it is recommended to adjust this setting accordingly.\n"],
 			'referrer-origin' => [['Referrer-Policy' => 'origin'], "- The `Referrer-Policy` HTTP header is not set to `no-referrer`, `no-referrer-when-downgrade`, `strict-origin`, `strict-origin-when-cross-origin` or `same-origin`. This can leak referer information. See the {w3c-recommendation}.\n"],
 			'referrer-origin-when-cross-origin' => [['Referrer-Policy' => 'origin-when-cross-origin'], "- The `Referrer-Policy` HTTP header is not set to `no-referrer`, `no-referrer-when-downgrade`, `strict-origin`, `strict-origin-when-cross-origin` or `same-origin`. This can leak referer information. See the {w3c-recommendation}.\n"],
 			'referrer-unsafe-url' => [['Referrer-Policy' => 'unsafe-url'], "- The `Referrer-Policy` HTTP header is not set to `no-referrer`, `no-referrer-when-downgrade`, `strict-origin`, `strict-origin-when-cross-origin` or `same-origin`. This can leak referer information. See the {w3c-recommendation}.\n"],
@@ -157,7 +157,7 @@ class SecurityHeadersTest extends TestCase {
 	public function testFailure(array $headers, string $msg): void {
 		$headers = array_merge(
 			[
-				'X-XSS-Protection' => '1; mode=block',
+				'X-XSS-Protection' => '0',
 				'X-Content-Type-Options' => 'nosniff',
 				'X-Robots-Tag' => 'noindex, nofollow',
 				'X-Frame-Options' => 'SAMEORIGIN',