CVE-2022-31128: Missing Authorization (CWE-862)
value: Tuleap is a Free & Open Source Suite to improve management of software developments and collaboration. In affected versions Tuleap does not properly verify permissions when creating branches with the REST API in Git repositories using the fine grained permissions. Users can create branches via the REST endpoint `POST git/:id/branches` regardless of the permissions set on the repository. This issue has been fixed in version 13.10.99.82 Tuleap Community Edition as well as in version 13.10-3 of Tuleap Enterprise Edition. Users are advised to upgrade. There are no known workarounds for this issue.
CWE description: The product does not perform an authorization check when an actor attempts to access a resource or perform an action.
CVSS v3 base score: 5.4
Published: 2022-08-01T17:15Z
Repository: https://github.com/Enalean/tuleap
Language: PHP
Files changed: plugins/git/include/REST/v1/Branch/BranchCreator.php, plugins/git/include/REST/v1/RepositoryResource.class.php, plugins/git/tests/unit/REST/v1/Branch/BranchCreatorTest.php
Commit date: 2022-07-05 17:49:53 +0200
Commit message: fix: Fine grained permissions must be well handled in POST git/:id/branches

This fixes request #27538 Fine grained permissions are not checked when creating a branch with REST API

The fine grained permissions were not taken into account in the previous
checks.

Change-Id: I1fa72ab34e4f41d5ea3ec9dd21e990ce330d780f

Diff: diff --git a/plugins/git/include/REST/v1/Branch/BranchCreator.php b/plugins/git/include/REST/v1/Branch/BranchCreator.php
index d24230fe0c1b..14390c13485d 100644
--- a/plugins/git/include/REST/v1/Branch/BranchCreator.php
+++ b/plugins/git/include/REST/v1/Branch/BranchCreator.php
@@ -19,42 +19,35 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
 namespace Tuleap\Git\REST\v1\Branch;
 
-use Git;
 use Git_Command_Exception;
 use Luracast\Restler\RestException;
 use Tuleap\Git\Branch\BranchCreationExecutor;
 use Tuleap\Git\Branch\BranchName;
 use Tuleap\Git\Branch\CannotCreateNewBranchException;
 use Tuleap\Git\Branch\InvalidBranchNameException;
+use Tuleap\Git\Permissions\AccessControlVerifier;
 use Tuleap\Git\REST\v1\GitBranchPOSTRepresentation;
 
 class BranchCreator
 {
     private const BRANCH_PREFIX = "refs/heads/";
 
-    public function __construct(private \Git_Exec $git_exec, private BranchCreationExecutor $branch_creation_executor)
-    {
+    public function __construct(
+        private \Git_Exec $git_exec,
+        private BranchCreationExecutor $branch_creation_executor,
+        private AccessControlVerifier $access_control_verifier,
+    ) {
     }
 
     /**
      * @throws RestException
      */
     public function createBranch(\PFUser $user, \GitRepository $repository, GitBranchPOSTRepresentation $representation): void
     {
-        if (
-            ! $user->hasPermission(Git::PERM_WRITE, $repository->getId(), $repository->getProjectId())
-            && ! $user->hasPermission(Git::PERM_WPLUS, $repository->getId(), $repository->getProjectId())
-        ) {
-            throw new RestException(
-                403,
-                "User cannot update the content of the repository"
-            );
-        }
-
         try {
             BranchName::fromBranchNameShortHand($representation->branch_name);
         } catch (InvalidBranchNameException $exception) {
             throw new RestException(
                 400,
@@ -63,10 +56,21 @@ class BranchCreator
                     $representation->branch_name
                 )
             );
         }
 
+        if (! $this->access_control_verifier->canWrite($user, $repository, self::BRANCH_PREFIX . $representation->branch_name)) {
+            throw new RestException(
+                403,
+                sprintf(
+                    "You are not allowed to create the branch %s in repository %s",
+                    $representation->branch_name,
+                    $repository->getName()
+                )
+            );
+        }
+
         $all_branches_names = $this->git_exec->getAllBranchesSortedByCreationDate();
         if (in_array($representation->branch_name, $all_branches_names)) {
             throw new RestException(
                 400,
                 sprintf(
diff --git a/plugins/git/include/REST/v1/RepositoryResource.class.php b/plugins/git/include/REST/v1/RepositoryResource.class.php
index 0b36c9428988..b82c553467fb 100644
--- a/plugins/git/include/REST/v1/RepositoryResource.class.php
+++ b/plugins/git/include/REST/v1/RepositoryResource.class.php
@@ -70,10 +70,11 @@ use Tuleap\Git\Exceptions\RepositoryNotMigratedException;
 use Tuleap\Git\Gitolite\GitoliteAccessURLGenerator;
 use Tuleap\Git\Gitolite\VersionDetector;
 use Tuleap\Git\GitPHP\ProjectProvider;
 use Tuleap\Git\GitPHP\RepositoryAccessException;
 use Tuleap\Git\GitPHP\RepositoryNotExistingException;
+use Tuleap\Git\Permissions\AccessControlVerifier;
 use Tuleap\Git\Permissions\DefaultFineGrainedPermissionFactory;
 use Tuleap\Git\Permissions\FineGrainedDao;
 use Tuleap\Git\Permissions\FineGrainedPatternValidator;
 use Tuleap\Git\Permissions\FineGrainedPermissionFactory;
 use Tuleap\Git\Permissions\FineGrainedPermissionReplicator;
@@ -917,11 +918,22 @@ class RepositoryResource extends AuthenticatedResource
         $this->checkAccess();
         $this->optionsGetPostBranches($id);
 
         $repository = $this->getRepositoryForCurrentUser($id);
 
-        (new BranchCreator(new Git_Exec($repository->getFullPath(), $repository->getFullPath()), new BranchCreationExecutor()))->createBranch(
+        $branch_creator = new BranchCreator(
+            new Git_Exec($repository->getFullPath(), $repository->getFullPath()),
+            new BranchCreationExecutor(),
+            new AccessControlVerifier(
+                new FineGrainedRetriever(
+                    new FineGrainedDao(),
+                ),
+                new \System_Command(),
+            )
+        );
+
+        $branch_creator->createBranch(
             $this->getCurrentUser(),
             $repository,
             $representation
         );
     }
diff --git a/plugins/git/tests/unit/REST/v1/Branch/BranchCreatorTest.php b/plugins/git/tests/unit/REST/v1/Branch/BranchCreatorTest.php
index 328dfd27c271..0bbfca486e82 100644
--- a/plugins/git/tests/unit/REST/v1/Branch/BranchCreatorTest.php
+++ b/plugins/git/tests/unit/REST/v1/Branch/BranchCreatorTest.php
@@ -21,16 +21,17 @@
 
 declare(strict_types=1);
 
 namespace Tuleap\Git\REST\v1\Branch;
 
-use Git;
 use Git_Command_Exception;
 use Luracast\Restler\RestException;
 use Tuleap\Git\Branch\BranchCreationExecutor;
 use Tuleap\Git\Branch\CannotCreateNewBranchException;
+use Tuleap\Git\Permissions\AccessControlVerifier;
 use Tuleap\Git\REST\v1\GitBranchPOSTRepresentation;
+use Tuleap\Test\Builders\UserTestBuilder;
 use Tuleap\Test\PHPUnit\TestCase;
 
 final class BranchCreatorTest extends TestCase
 {
     private const REPO_ID    = 1;
@@ -43,21 +44,27 @@ final class BranchCreatorTest extends TestCase
     private $git_exec;
     /**
      * @var \PHPUnit\Framework\MockObject\MockObject&BranchCreationExecutor
      */
     private $branch_creation_executor;
+    /**
+     * @var AccessControlVerifier&\PHPUnit\Framework\MockObject\MockObject
+     */
+    private $access_control_verifier;
 
     protected function setUp(): void
     {
         parent::setUp();
 
         $this->git_exec                 = $this->createMock(\Git_Exec::class);
         $this->branch_creation_executor = $this->createMock(BranchCreationExecutor::class);
+        $this->access_control_verifier  = $this->createMock(AccessControlVerifier::class);
 
         $this->creator = new BranchCreator(
             $this->git_exec,
-            $this->branch_creation_executor
+            $this->branch_creation_executor,
+            $this->access_control_verifier
         );
 
         $this->git_exec->method('getAllBranchesSortedByCreationDate')->willReturn([
             "main",
             "existing_branch_01",
@@ -88,11 +95,11 @@ final class BranchCreatorTest extends TestCase
     {
         $this->branch_creation_executor->expects(self::never())
             ->method("createNewBranch");
 
         $this->expectException(RestException::class);
-        $this->expectExceptionMessage("User cannot update the content of the repository");
+        $this->expectExceptionMessage("You are not allowed to create the branch new_branch in repository repo01");
 
         $this->creator->createBranch(
             $this->buildMockUserWithoutPermissions(),
             $this->buildMockGitRepository(),
             GitBranchPOSTRepresentation::build(
@@ -199,26 +206,20 @@ final class BranchCreatorTest extends TestCase
         );
     }
 
     private function buildMockUserWithPermissions(): \PFUser
     {
-        $user = $this->createMock(\PFUser::class);
-        $user->method('hasPermission')->willReturnMap([
-            [Git::PERM_WRITE, self::REPO_ID, self::PROJECT_ID, true],
-            [Git::PERM_WPLUS, self::REPO_ID, self::PROJECT_ID, true],
-        ]);
+        $user = UserTestBuilder::anActiveUser()->build();
+        $this->access_control_verifier->method('canWrite')->willReturn(true);
 
         return $user;
     }
 
     private function buildMockUserWithoutPermissions(): \PFUser
     {
-        $user = $this->createMock(\PFUser::class);
-        $user->method('hasPermission')->willReturnMap([
-            [Git::PERM_WRITE, self::REPO_ID, self::PROJECT_ID, false],
-            [Git::PERM_WPLUS, self::REPO_ID, self::PROJECT_ID, false],
-        ]);
+        $user = UserTestBuilder::anActiveUser()->build();
+        $this->access_control_verifier->method('canWrite')->willReturn(false);
 
         return $user;
     }
 
     private function buildMockGitRepository(): \GitRepository

Vulnerable code: use Git;
    public function __construct(private \Git_Exec $git_exec, private BranchCreationExecutor $branch_creation_executor)
    {
        if (
            ! $user->hasPermission(Git::PERM_WRITE, $repository->getId(), $repository->getProjectId())
            && ! $user->hasPermission(Git::PERM_WPLUS, $repository->getId(), $repository->getProjectId())
        ) {
            throw new RestException(
                403,
                "User cannot update the content of the repository"
            );
        }

        (new BranchCreator(new Git_Exec($repository->getFullPath(), $repository->getFullPath()), new BranchCreationExecutor()))->createBranch(
use Git;
            $this->branch_creation_executor
        $this->expectExceptionMessage("User cannot update the content of the repository");
        $user = $this->createMock(\PFUser::class);
        $user->method('hasPermission')->willReturnMap([
            [Git::PERM_WRITE, self::REPO_ID, self::PROJECT_ID, true],
            [Git::PERM_WPLUS, self::REPO_ID, self::PROJECT_ID, true],
        ]);
        $user = $this->createMock(\PFUser::class);
        $user->method('hasPermission')->willReturnMap([
            [Git::PERM_WRITE, self::REPO_ID, self::PROJECT_ID, false],
            [Git::PERM_WPLUS, self::REPO_ID, self::PROJECT_ID, false],
        ]);
Fixed code: use Tuleap\Git\Permissions\AccessControlVerifier;
    public function __construct(
        private \Git_Exec $git_exec,
        private BranchCreationExecutor $branch_creation_executor,
        private AccessControlVerifier $access_control_verifier,
    ) {
        if (! $this->access_control_verifier->canWrite($user, $repository, self::BRANCH_PREFIX . $representation->branch_name)) {
            throw new RestException(
                403,
                sprintf(
                    "You are not allowed to create the branch %s in repository %s",
                    $representation->branch_name,
                    $repository->getName()
                )
            );
        }

use Tuleap\Git\Permissions\AccessControlVerifier;
        $branch_creator = new BranchCreator(
            new Git_Exec($repository->getFullPath(), $repository->getFullPath()),
            new BranchCreationExecutor(),
            new AccessControlVerifier(
                new FineGrainedRetriever(
                    new FineGrainedDao(),
                ),
                new \System_Command(),
            )
        );

        $branch_creator->createBranch(
use Tuleap\Git\Permissions\AccessControlVerifier;
use Tuleap\Test\Builders\UserTestBuilder;
    /**
     * @var AccessControlVerifier&\PHPUnit\Framework\MockObject\MockObject
     */
    private $access_control_verifier;
        $this->access_control_verifier  = $this->createMock(AccessControlVerifier::class);
            $this->branch_creation_executor,
            $this->access_control_verifier
        $this->expectExceptionMessage("You are not allowed to create the branch new_branch in repository repo01");
        $user = UserTestBuilder::anActiveUser()->build();
        $this->access_control_verifier->method('canWrite')->willReturn(true);
        $user = UserTestBuilder::anActiveUser()->build();
        $this->access_control_verifier->method('canWrite')->willReturn(false);