From 9160fdc6835f0b18ab12c676b7a8224cd4fd4a01 Mon Sep 17 00:00:00 2001 From: MichaIng Date: Sat, 28 Jun 2025 01:12:31 +0200 Subject: [PATCH] fix(settings): update X-XSS-Protection recommendation 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 --- apps/settings/lib/SetupChecks/SecurityHeaders.php | 4 ++-- .../tests/SetupChecks/SecurityHeadersTest.php | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/apps/settings/lib/SetupChecks/SecurityHeaders.php b/apps/settings/lib/SetupChecks/SecurityHeaders.php index ed4e56218da58..62e2d7535fffc 100644 --- a/apps/settings/lib/SetupChecks/SecurityHeaders.php +++ b/apps/settings/lib/SetupChecks/SecurityHeaders.php @@ -73,8 +73,8 @@ public function run(): SetupResult { } $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 e7d87775485eb..83a72f262c5a8 100644 --- a/apps/settings/tests/SetupChecks/SecurityHeadersTest.php +++ b/apps/settings/tests/SetupChecks/SecurityHeadersTest.php @@ -84,7 +84,7 @@ public function testSomeHeadersMissing(): void { $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 @@ public static function dataSuccess(): array { 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 @@ public static function dataSuccess(): array { 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 @@ public static function dataFailure(): array { 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 @@ public static function dataFailure(): array { 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',