Uploaded image for project: 'eZ Publish / Platform'
  1. eZ Publish / Platform
  2. EZP-31782

PermissionChecker caches policy limitations incorrectly

    XMLWordPrintable

    Details

    • Sprint:
      [3.2] - Sprint 4

      Description

      This must be fixed before https://jira.ez.no/browse/EZP-31783 / https://github.com/ezsystems/ezplatform-admin-ui/pull/1441 can be fixed.

      Impact

      PermissionChecker cache for policy limitations causes issues. (This is not a security issue, as security checks happen elsewhere and are unaffected by this. This cache only gives a hint about what is allowed, and errors here cannot be exploited to get unauthorized access.)

      The simplest use case to reproduce the issue:

      1. Install clean eZ Platform 3.1.1
      2. Create a "Test" user role:
        User Function Limitations
        User Login None
        Content Read None
        Content Create Content Type: Article
      1. Create "Test" user group and assign "Test" user role to it.
      2. Create a new user in "Test" group
      3. Create a following event subscriber in src/EventSubscriber/AdvancedUserPermissions.php:

       

      {{<?php

      declare(strict_types=1);

      namespace App\EventSubscriber;

      use eZ\Publish\API\Repository\PermissionResolver;
      use eZ\Publish\API\Repository\Values\User\Limitation\ContentTypeLimitation;
      use EzSystems\EzPlatformAdminUi\Permission\PermissionCheckerInterface;
      use Symfony\Component\EventDispatcher\EventSubscriberInterface;

      class AdvancedUserPermissions implements EventSubscriberInterface
      {
      /** @var PermissionResolver */
      private $permissionResolver;

      /** @var PermissionCheckerInterface */
      private $permissionChecker;

      public function __construct(
      PermissionResolver $permissionResolver,
      PermissionCheckerInterface $permissionChecker
      )

      { $this->permissionResolver = $permissionResolver; $this->permissionChecker = $permissionChecker; }

      public static function getSubscribedEvents(): array

      { return [ 'kernel.request' => ['fixRestrictions', 99999], ]; }

      public function fixRestrictions(): void
      {
      $readAccess = $this->permissionResolver->hasAccess('content', 'read');
      if (is_array($readAccess))

      { // Store in cache section limitation for content/read function and it will be reused for all other functions $this->permissionChecker->getRestrictions($readAccess, ContentTypeLimitation::class); }

      $createAccess = $this->permissionResolver->hasAccess('content', 'create');
      if (is_array($createAccess))

      { var_dump($this->permissionChecker->getRestrictions($createAccess, ContentTypeLimitation::class)); exit(); }

      }
      }}}

      1. Clear the caches.
      2. Login using new user credentials (step #4).
      3. You will see that permissionChecker will return empty content type limitation for content/create function. And does not match step #2.

      Right now permissionChecker checker is used only in some places of admin UI (not sure if there are any plans to reuse it in other parts). I didn't spend a lot of time for trying to find a simple exploited for this. But most likely in some cases, it is possible to exploit in out of the box install.

      Problem summary:
      Restrictions are cached after the first call of permissionChecker::getRestrictions. And the same restrictions are reused for further permissionChecker::getRestrictions calls, even for different modules/functions.

      Patches

      Suggested patch:

       

      {{$ git diff src/lib/Permission/PermissionChecker.php
      diff --git a/src/lib/Permission/PermissionChecker.php b/src/lib/Permission/PermissionChecker.php
      index 579918a0..24c486c1 100644
      — a/src/lib/Permission/PermissionChecker.php
      +++ b/src/lib/Permission/PermissionChecker.php
      @@ -234,10 +234,10 @@ class PermissionChecker implements PermissionCheckerInterface
      */
      private function flattenArrayOfLimitationsForCurrentUser(array $hasAccess): array
      {

      • $currentUserId = $this->permissionResolver->getCurrentUserReference()->getUserId();
        + $cacheKey = $this->getFlattenArrayOfLimitationsCacheKey($hasAccess);
      • if ($this->flattenArrayOfLimitations !== null && is_array($this->flattenArrayOfLimitations[$currentUserId])) { - return $this->flattenArrayOfLimitations[$currentUserId]; + if ($this->flattenArrayOfLimitations !== null && isset($this->flattenArrayOfLimitations[$cacheKey])) \{ + return $this->flattenArrayOfLimitations[$cacheKey]; }

      $limitations = [];
      @@ -256,7 +256,31 @@ class PermissionChecker implements PermissionCheckerInterface
      }
      }

      • return $this->flattenArrayOfLimitations[$currentUserId] = $limitations;
        + return $this->flattenArrayOfLimitations[$cacheKey] = $limitations;
        + }
        +
        + /**
        + * @param array $hasAccess
        + *
        + * @return string
        + */
        + private function getFlattenArrayOfLimitationsCacheKey(array $hasAccess): string
        + {
        + $moduleFunctions = [];
        + foreach ($hasAccess as $permissionSet)
        Unknown macro: { + /** @var eZPublishAPIRepositoryValuesUserPolicy $policy */ + foreach ($permissionSet['policies'] as $policy)Unknown macro}

        + }
        + sort($moduleFunctions);
        +
        + $currentUserId = $this->permissionResolver->getCurrentUserReference()->getUserId();
        +
        + return implode(';', [$currentUserId, ...$moduleFunctions]);
        }

      /**}}

        Attachments

          Activity

            People

            Assignee:
            Unassigned
            Reporter:
            gunnstein.lye@ez.no Gunnstein Lye
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: