CVE-2023-39343: Observable Response Discrepancy (CWE-204)
value: Sulu is an open-source PHP content management system based on the Symfony framework. It allows over the Admin Login form to detect which user (username, email) exists and which one do not exist. Sulu Installation not using the old Symfony 5.4 security System and previous version are not impacted by this Security issue. The vulnerability has been patched in version 2.5.10. 


CWE description: The product provides different responses to incoming requests in a way that reveals internal state information to an unauthorized actor outside of the intended control sphere.
CVSS v3 base score: 4.3
Published: 2023-08-04T01:15Z
Repository: https://github.com/sulu/sulu
Language: PHP
Files changed: config/packages/security.yaml, src/Sulu/Bundle/SecurityBundle/Security/AuthenticationHandler.php, src/Sulu/Bundle/SecurityBundle/Tests/Application/config/config.yml, src/Sulu/Bundle/SecurityBundle/Tests/Functional/Security/AuthenticationHandlerTest.php
Commit date: 2023-08-03 20:02:42 +0200
Commit message: Merge pull request from GHSA-wmwf-49vv-p3mr

* Fix AuthenticationHandler return inner exception message

* Add funcctional test case for AuthenticationHandlerTest

* Sync security bundle and sulu skeleton security config

* Remove not longer used LoginControllerTest via form_login

* Add test for success and fail login
Diff: diff --git a/config/packages/security.yaml b/config/packages/security.yaml
index be39e46f5c..ae669245cc 100644
--- a/config/packages/security.yaml
+++ b/config/packages/security.yaml
@@ -18,18 +18,17 @@ security:
     # Note: Only the *first* access control that matches will be used
     access_control:
         - { path: ^/admin/reset, roles: PUBLIC_ACCESS }
         - { path: ^/admin/security/reset, roles: PUBLIC_ACCESS }
         - { path: ^/admin/login$, roles: PUBLIC_ACCESS }
+        - { path: ^/admin/2fa, roles: PUBLIC_ACCESS }
         - { path: ^/admin/_wdt, roles: PUBLIC_ACCESS }
+        - { path: ^/admin/_profiler, roles: PUBLIC_ACCESS }
         - { path: ^/admin/translations, roles: PUBLIC_ACCESS }
         - { path: ^/admin$, roles: PUBLIC_ACCESS }
         - { path: ^/admin/$, roles: PUBLIC_ACCESS }
         - { path: ^/admin/p/, roles: PUBLIC_ACCESS }
-        - { path: ^/admin/logout, role: PUBLIC_ACCESS }
-        - { path: ^/admin/_profiler, role: PUBLIC_ACCESS }
-        - { path: ^/admin/2fa, role: PUBLIC_ACCESS }
         - { path: ^/admin, roles: ROLE_USER }
 
     firewalls:
         dev:
             pattern: ^/(_(profiler|wdt)|css|images|js)/
diff --git a/src/Sulu/Bundle/SecurityBundle/Security/AuthenticationHandler.php b/src/Sulu/Bundle/SecurityBundle/Security/AuthenticationHandler.php
index 6054a2a488..8d4efb8f70 100644
--- a/src/Sulu/Bundle/SecurityBundle/Security/AuthenticationHandler.php
+++ b/src/Sulu/Bundle/SecurityBundle/Security/AuthenticationHandler.php
@@ -102,11 +102,11 @@ class AuthenticationHandler implements AuthenticationSuccessHandlerInterface, Au
      */
     public function onAuthenticationFailure(Request $request, AuthenticationException $exception): Response
     {
         if ($request->isXmlHttpRequest()) {
             // if AJAX login
-            $array = ['message' => $exception->getMessage()];
+            $array = ['message' => $exception->getMessageKey()];
             $response = new JsonResponse($array, 401);
         } else {
             // if form login
             // set authentication exception to session
             $request->getSession()->set(Security::AUTHENTICATION_ERROR, $exception);
diff --git a/src/Sulu/Bundle/SecurityBundle/Tests/Application/config/config.yml b/src/Sulu/Bundle/SecurityBundle/Tests/Application/config/config.yml
index a9a3cd20d5..6b9b5efbba 100644
--- a/src/Sulu/Bundle/SecurityBundle/Tests/Application/config/config.yml
+++ b/src/Sulu/Bundle/SecurityBundle/Tests/Application/config/config.yml
@@ -6,26 +6,31 @@ security:
     enable_authenticator_manager: true
 
     access_control:
         - { path: ^/admin/reset, roles: PUBLIC_ACCESS }
         - { path: ^/admin/security/reset, roles: PUBLIC_ACCESS }
-        - { path: ^/admin/login, roles: PUBLIC_ACCESS }
-        - { path: ^/admin/2fa, role: PUBLIC_ACCESS }
+        - { path: ^/admin/login$, roles: PUBLIC_ACCESS }
+        - { path: ^/admin/2fa, roles: PUBLIC_ACCESS }
+        - { path: ^/admin/_wdt, roles: PUBLIC_ACCESS }
+        - { path: ^/admin/_profiler, roles: PUBLIC_ACCESS }
+        - { path: ^/admin/translations, roles: PUBLIC_ACCESS }
+        - { path: ^/admin$, roles: PUBLIC_ACCESS }
+        - { path: ^/admin/$, roles: PUBLIC_ACCESS }
+        - { path: ^/admin/p/, roles: PUBLIC_ACCESS }
         - { path: ^/admin, roles: ROLE_USER }
 
     firewalls:
         test:
             pattern: ^/
             lazy: true
             entry_point: sulu_security.authentication_entry_point
-            form_login:
+            json_login:
                 check_path: sulu_admin.login_check
                 success_handler: sulu_security.authentication_handler
                 failure_handler: sulu_security.authentication_handler
             logout:
-                path: /admin/logout
-                target: /admin/
+                path: sulu_admin.logout
             two_factor:
                 prepare_on_login: true
                 prepare_on_access_denied: true
                 check_path: 2fa_login_check_admin
                 authentication_required_handler: sulu_security.two_factor_authentication_required_handler
diff --git a/src/Sulu/Bundle/SecurityBundle/Tests/Functional/Controller/LoginControllerTest.php b/src/Sulu/Bundle/SecurityBundle/Tests/Functional/Controller/LoginControllerTest.php
deleted file mode 100644
index dbcfbe26f3..0000000000
--- a/src/Sulu/Bundle/SecurityBundle/Tests/Functional/Controller/LoginControllerTest.php
+++ /dev/null
@@ -1,29 +0,0 @@
-<?php
-
-/*
- * This file is part of Sulu.
- *
- * (c) Sulu GmbH
- *
- * This source file is subject to the MIT license that is bundled
- * with this source code in the file LICENSE.
- */
-
-namespace Sulu\Bundle\AdminBundle\Tests\Functional\Controller;
-
-use Sulu\Bundle\TestBundle\Testing\SuluTestCase;
-
-class LoginControllerTest extends SuluTestCase
-{
-    public function testFalseLoginRedirect(): void
-    {
-        $client = $this->createClient();
-        $client->request('POST', '/admin/login', [
-            '_username' => 'FalseUser',
-            '_password' => 'FalsePassword',
-        ]);
-        $response = $client->getResponse();
-        $this->assertHttpStatusCode(302, $response);
-        $this->assertSame('/admin/', $response->getTargetUrl());
-    }
-}
diff --git a/src/Sulu/Bundle/SecurityBundle/Tests/Functional/Security/AuthenticationHandlerTest.php b/src/Sulu/Bundle/SecurityBundle/Tests/Functional/Security/AuthenticationHandlerTest.php
new file mode 100644
index 0000000000..5d3f359857
--- /dev/null
+++ b/src/Sulu/Bundle/SecurityBundle/Tests/Functional/Security/AuthenticationHandlerTest.php
@@ -0,0 +1,82 @@
+<?php
+
+/*
+ * This file is part of Sulu.
+ *
+ * (c) Sulu GmbH
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Sulu\Bundle\SecurityBundle\Tests\Functional\Security;
+
+use Sulu\Bundle\TestBundle\Testing\SuluTestCase;
+
+class AuthenticationHandlerTest extends SuluTestCase
+{
+    public function testLoginFail(): void
+    {
+        $client = $this->createClient([], [
+            'CONTENT_TYPE' => 'application/json',
+            'HTTP_ACCEPT' => 'application/json',
+            'HTTP_X-Requested-With' => 'XMLHttpRequest',
+        ]);
+
+        $client->request('POST', '/admin/login', [], [], [], '{"username": "not-existing-user", "password": "wrong"}');
+
+        $response = $client->getResponse();
+        $this->assertHttpStatusCode(401, $response);
+        $notExistUserContent = $response->getContent();
+
+        $this->assertSame('{"message":"Invalid credentials."}', $notExistUserContent);
+    }
+
+    public function testLoginSuccess(): void
+    {
+        $client = $this->createClient([], [
+            'CONTENT_TYPE' => 'application/json',
+            'HTTP_ACCEPT' => 'application/json',
+            'HTTP_X-Requested-With' => 'XMLHttpRequest',
+        ]);
+
+        $testUser = $this->getTestUser();
+
+        $client->request('POST', '/admin/login', [], [], [], '{"username": "' . $testUser->getUsername() . '", "password": "test"}');
+
+        $response = $client->getResponse();
+        $this->assertHttpStatusCode(200, $response);
+        $notExistUserContent = $response->getContent();
+
+        $this->assertSame(
+            '{"url":"\/admin\/","username":"test","completed":true,"twoFactorMethods":["trusted_devices"]}',
+            $notExistUserContent
+        );
+    }
+
+    public function testLoginFailExistUserHasSameMessageAsNotExist(): void
+    {
+        $client = $this->createClient([], [
+            'CONTENT_TYPE' => 'application/json',
+            'HTTP_ACCEPT' => 'application/json',
+            'HTTP_X-Requested-With' => 'XMLHttpRequest',
+        ]);
+
+        $testUser = $this->getTestUser();
+
+        $client->request('POST', '/admin/login', [], [], [], '{"username": "not-existing-user", "password": "wrong"}');
+
+        $response = $client->getResponse();
+        $this->assertHttpStatusCode(401, $response);
+        $notExistUserContent = $response->getContent();
+
+        $client->request('POST', '/admin/login', [], [], [], '{"username": "' . $testUser->getUsername() . '", "password": "wrong"}');
+
+        $response = $client->getResponse();
+        $this->assertHttpStatusCode(401, $response);
+        $existUserContent = $response->getContent();
+
+        $this->assertSame($notExistUserContent, $existUserContent);
+        $this->assertSame('{"message":"Invalid credentials."}', $notExistUserContent);
+    }
+}

Vulnerable code:         - { path: ^/admin/logout, role: PUBLIC_ACCESS }
        - { path: ^/admin/_profiler, role: PUBLIC_ACCESS }
        - { path: ^/admin/2fa, role: PUBLIC_ACCESS }
            $array = ['message' => $exception->getMessage()];
        - { path: ^/admin/login, roles: PUBLIC_ACCESS }
        - { path: ^/admin/2fa, role: PUBLIC_ACCESS }
            form_login:
                path: /admin/logout
                target: /admin/
<?php

/*
 * This file is part of Sulu.
 *
 * (c) Sulu GmbH
 *
 * This source file is subject to the MIT license that is bundled
 * with this source code in the file LICENSE.
 */

namespace Sulu\Bundle\AdminBundle\Tests\Functional\Controller;

use Sulu\Bundle\TestBundle\Testing\SuluTestCase;

class LoginControllerTest extends SuluTestCase
{
    public function testFalseLoginRedirect(): void
    {
        $client = $this->createClient();
        $client->request('POST', '/admin/login', [
            '_username' => 'FalseUser',
            '_password' => 'FalsePassword',
        ]);
        $response = $client->getResponse();
        $this->assertHttpStatusCode(302, $response);
        $this->assertSame('/admin/', $response->getTargetUrl());
    }
}
Fixed code:         - { path: ^/admin/2fa, roles: PUBLIC_ACCESS }
        - { path: ^/admin/_profiler, roles: PUBLIC_ACCESS }
            $array = ['message' => $exception->getMessageKey()];
        - { path: ^/admin/login$, roles: PUBLIC_ACCESS }
        - { path: ^/admin/2fa, roles: PUBLIC_ACCESS }
        - { path: ^/admin/_wdt, roles: PUBLIC_ACCESS }
        - { path: ^/admin/_profiler, roles: PUBLIC_ACCESS }
        - { path: ^/admin/translations, roles: PUBLIC_ACCESS }
        - { path: ^/admin$, roles: PUBLIC_ACCESS }
        - { path: ^/admin/$, roles: PUBLIC_ACCESS }
        - { path: ^/admin/p/, roles: PUBLIC_ACCESS }
            json_login:
                path: sulu_admin.logout
<?php

/*
 * This file is part of Sulu.
 *
 * (c) Sulu GmbH
 *
 * This source file is subject to the MIT license that is bundled
 * with this source code in the file LICENSE.
 */

namespace Sulu\Bundle\SecurityBundle\Tests\Functional\Security;

use Sulu\Bundle\TestBundle\Testing\SuluTestCase;

class AuthenticationHandlerTest extends SuluTestCase
{
    public function testLoginFail(): void
    {
        $client = $this->createClient([], [
            'CONTENT_TYPE' => 'application/json',
            'HTTP_ACCEPT' => 'application/json',
            'HTTP_X-Requested-With' => 'XMLHttpRequest',
        ]);

        $client->request('POST', '/admin/login', [], [], [], '{"username": "not-existing-user", "password": "wrong"}');

        $response = $client->getResponse();
        $this->assertHttpStatusCode(401, $response);
        $notExistUserContent = $response->getContent();

        $this->assertSame('{"message":"Invalid credentials."}', $notExistUserContent);
    }

    public function testLoginSuccess(): void
    {
        $client = $this->createClient([], [
            'CONTENT_TYPE' => 'application/json',
            'HTTP_ACCEPT' => 'application/json',
            'HTTP_X-Requested-With' => 'XMLHttpRequest',
        ]);

        $testUser = $this->getTestUser();

        $client->request('POST', '/admin/login', [], [], [], '{"username": "' . $testUser->getUsername() . '", "password": "test"}');

        $response = $client->getResponse();
        $this->assertHttpStatusCode(200, $response);
        $notExistUserContent = $response->getContent();

        $this->assertSame(
            '{"url":"\/admin\/","username":"test","completed":true,"twoFactorMethods":["trusted_devices"]}',
            $notExistUserContent
        );
    }

    public function testLoginFailExistUserHasSameMessageAsNotExist(): void
    {
        $client = $this->createClient([], [
            'CONTENT_TYPE' => 'application/json',
            'HTTP_ACCEPT' => 'application/json',
            'HTTP_X-Requested-With' => 'XMLHttpRequest',
        ]);

        $testUser = $this->getTestUser();

        $client->request('POST', '/admin/login', [], [], [], '{"username": "not-existing-user", "password": "wrong"}');

        $response = $client->getResponse();
        $this->assertHttpStatusCode(401, $response);
        $notExistUserContent = $response->getContent();

        $client->request('POST', '/admin/login', [], [], [], '{"username": "' . $testUser->getUsername() . '", "password": "wrong"}');

        $response = $client->getResponse();
        $this->assertHttpStatusCode(401, $response);
        $existUserContent = $response->getContent();

        $this->assertSame($notExistUserContent, $existUserContent);
        $this->assertSame('{"message":"Invalid credentials."}', $notExistUserContent);
    }
}