CVE-2021-43835: Improper Privilege Management (CWE-269)
value: Sulu is an open-source PHP content management system based on the Symfony framework. In affected versions Sulu users who have access to any subset of the admin UI are able to elevate their privilege. Over the API it was possible for them to give themselves permissions to areas which they did not already had. This issue was introduced in 2.0.0-RC1 with the new ProfileController putAction. The versions have been patched in 2.2.18, 2.3.8 and 2.4.0. For users unable to upgrade the only known workaround is to apply a patch to the ProfileController manually.
CWE description: The product does not properly assign, modify, track, or check privileges for an actor, creating an unintended sphere of control for that actor.
CVSS v3 base score: 7.2
Published: 2021-12-15T20:15Z
Repository: https://github.com/sulu/sulu
Language: PHP
Files changed: src/Sulu/Bundle/SecurityBundle/Controller/ProfileController.php, src/Sulu/Bundle/SecurityBundle/Tests/Functional/Controller/ProfileControllerTest.php
Commit date: 2021-12-15 14:49:33 +0100
Commit message: Merge pull request from GHSA-84px-q68r-2fc9


Diff: diff --git a/src/Sulu/Bundle/SecurityBundle/Controller/ProfileController.php b/src/Sulu/Bundle/SecurityBundle/Controller/ProfileController.php
index 3aadf72df6..098bb81873 100644
--- a/src/Sulu/Bundle/SecurityBundle/Controller/ProfileController.php
+++ b/src/Sulu/Bundle/SecurityBundle/Controller/ProfileController.php
@@ -116,11 +116,11 @@ class ProfileController implements ClassResourceInterface
      */
     public function putAction(Request $request)
     {
         $this->checkArguments($request);
         $user = $this->tokenStorage->getToken()->getUser();
-        $this->userManager->save($request->request->all(), $request->get('locale'), $user->getId(), true);
+        $this->userManager->save($this->getData($request), $request->get('locale'), $user->getId(), true);
 
         $user->setFirstName($request->get('firstName'));
         $user->setLastName($request->get('lastName'));
 
         $this->objectManager->flush();
@@ -229,6 +229,22 @@ class ProfileController implements ClassResourceInterface
         }
         if (null === $request->get('locale')) {
             throw new MissingArgumentException($this->userClass, 'locale');
         }
     }
+
+    /**
+     * @return array<string, mixed>
+     */
+    protected function getData(Request $request): array
+    {
+        $data = [];
+
+        foreach ($request->request->all() as $key => $value) {
+            if (\in_array($key, ['firstName', 'lastName', 'username', 'email', 'password', 'locale'], true)) {
+                $data[$key] = $value;
+            }
+        }
+
+        return $data;
+    }
 }
diff --git a/src/Sulu/Bundle/SecurityBundle/Tests/Functional/Controller/ProfileControllerTest.php b/src/Sulu/Bundle/SecurityBundle/Tests/Functional/Controller/ProfileControllerTest.php
index fd6dff0e49..57f5d79952 100644
--- a/src/Sulu/Bundle/SecurityBundle/Tests/Functional/Controller/ProfileControllerTest.php
+++ b/src/Sulu/Bundle/SecurityBundle/Tests/Functional/Controller/ProfileControllerTest.php
@@ -85,10 +85,34 @@ class ProfileControllerTest extends SuluTestCase
         $this->assertEquals('hansi', $response->username);
         $this->assertEquals('hans.mustermann@muster.at', $response->email);
         $this->assertEquals('de', $response->locale);
     }
 
+    public function testPutInvalidField()
+    {
+        $this->client->jsonRequest(
+            'PUT',
+            '/api/profile',
+            [
+                'firstName' => 'Hans',
+                'lastName' => 'Mustermann',
+                'username' => 'hansi',
+                'email' => 'hans.mustermann@muster.at',
+                'password' => 'testpassword',
+                'locale' => 'de',
+            ]
+        );
+
+        $response = \json_decode($this->client->getResponse()->getContent());
+        $this->assertHttpStatusCode(200, $this->client->getResponse());
+        $this->assertEquals('Hans', $response->firstName);
+        $this->assertEquals('Mustermann', $response->lastName);
+        $this->assertEquals('hansi', $response->username);
+        $this->assertEquals('hans.mustermann@muster.at', $response->email);
+        $this->assertEquals('de', $response->locale);
+    }
+
     public function testPutEmailNotUnique()
     {
         $existingContact = new Contact();
         $existingContact->setFirstName('Max');
         $existingContact->setLastName('Muster');

Vulnerable code:         $this->userManager->save($request->request->all(), $request->get('locale'), $user->getId(), true);
Fixed code:         $this->userManager->save($this->getData($request), $request->get('locale'), $user->getId(), true);

    /**
     * @return array<string, mixed>
     */
    protected function getData(Request $request): array
    {
        $data = [];

        foreach ($request->request->all() as $key => $value) {
            if (\in_array($key, ['firstName', 'lastName', 'username', 'email', 'password', 'locale'], true)) {
                $data[$key] = $value;
            }
        }

        return $data;
    }
    public function testPutInvalidField()
    {
        $this->client->jsonRequest(
            'PUT',
            '/api/profile',
            [
                'firstName' => 'Hans',
                'lastName' => 'Mustermann',
                'username' => 'hansi',
                'email' => 'hans.mustermann@muster.at',
                'password' => 'testpassword',
                'locale' => 'de',
            ]
        );

        $response = \json_decode($this->client->getResponse()->getContent());
        $this->assertHttpStatusCode(200, $this->client->getResponse());
        $this->assertEquals('Hans', $response->firstName);
        $this->assertEquals('Mustermann', $response->lastName);
        $this->assertEquals('hansi', $response->username);
        $this->assertEquals('hans.mustermann@muster.at', $response->email);
        $this->assertEquals('de', $response->locale);
    }