endless loop detection for authz expressions

src/Authorization/AuthorizationDataMuxer.php

@@ -129,10 +129,14 @@ class AuthorizationDataMuxer
         $event = new GetAttributeEvent($this, $attributeName, $value, $userIdentifier);
         $event->setAttributeValue($value);
 
-        // Avoid endless recursions by only emitting an event for each attribute only once
-        if (!in_array($attributeName, $this->attributeStack, true)) {
-            array_push($this->attributeStack, $attributeName);
+        // Prevent endless recursions by only emitting an event for each attribute only once
+        if (in_array($attributeName, $this->attributeStack, true)) {
+            throw new AuthorizationException(sprintf('infinite loop caused by a %s subscriber. authorization attribute: %s', GetAttributeEvent::class, $attributeName), AuthorizationException::INFINITE_EVENT_LOOP_DETECTED);
+        }
+        array_push($this->attributeStack, $attributeName);
+        try {
             $this->eventDispatcher->dispatch($event);
+        } finally {
             array_pop($this->attributeStack);
         }
 

src/Authorization/AuthorizationException.php

@@ -6,7 +6,8 @@ namespace Dbp\Relay\CoreBundle\Authorization;
 
 class AuthorizationException extends \RuntimeException
 {
-    public const PRIVILEGE_UNDEFINED = 2;
-    public const ATTRIBUTE_UNDEFINED = 3;
-    public const INFINITE_LOOP_DETECTED = 4;
+    public const PRIVILEGE_UNDEFINED = 1;
+    public const ATTRIBUTE_UNDEFINED = 2;
+    public const INFINITE_EXRPESSION_LOOP_DETECTED = 3;
+    public const INFINITE_EVENT_LOOP_DETECTED = 4;
 }

src/Authorization/AuthorizationExpressionChecker.php

@@ -31,6 +31,12 @@ class AuthorizationExpressionChecker
     /** @var AuthorizationDataMuxer */
     private $dataMux;
 
+    /** @var array */
+    private $rightExpressionStack;
+
+    /** @var array */
+    private $attributeExpressionStack;
+
     public function __construct(AuthorizationDataMuxer $dataMux)
     {
         $this->expressionLanguage = new ExpressionLanguage();
@@ -38,6 +44,8 @@ class AuthorizationExpressionChecker
         $this->rightExpressions = [];
         $this->attributeExpressions = [];
         $this->dataMux = $dataMux;
+        $this->rightExpressionStack = [];
+        $this->attributeExpressionStack = [];
     }
 
     public function setConfig(array $config)
@@ -58,17 +66,24 @@ class AuthorizationExpressionChecker
      */
     public function evalAttributeExpression(AuthorizationUser $currentAuthorizationUser, string $expressionName, $defaultValue = null)
     {
-        $this->tryIncreaseRecursionCounter($expressionName);
-
-        if (($expression = $this->attributeExpressions[$expressionName] ?? null) !== null) {
-            $result = $this->expressionLanguage->evaluate($expression, [
-                'user' => $currentAuthorizationUser,
-            ]);
-        } else {
-            throw new AuthorizationException(sprintf('expression \'%s\' undefined', $expressionName), AuthorizationException::ATTRIBUTE_UNDEFINED);
+        if (in_array($expressionName, $this->attributeExpressionStack, true)) {
+            throw new AuthorizationException(sprintf('infinite loop caused by authorization attribute expression %s detected', $expressionName), AuthorizationException::INFINITE_EXRPESSION_LOOP_DETECTED);
+        }
+        array_push($this->attributeExpressionStack, $expressionName);
+
+        try {
+            if (($expression = $this->attributeExpressions[$expressionName] ?? null) !== null) {
+                $result = $this->expressionLanguage->evaluate($expression, [
+                    'user' => $currentAuthorizationUser,
+                ]);
+            } else {
+                throw new AuthorizationException(sprintf('expression \'%s\' undefined', $expressionName), AuthorizationException::ATTRIBUTE_UNDEFINED);
+            }
+
+            return $result ?? $defaultValue;
+        } finally {
+            array_pop($this->attributeExpressionStack);
         }
-
-        return $result ?? $defaultValue;
     }
 
     /**
@@ -91,17 +106,24 @@ class AuthorizationExpressionChecker
      */
     public function isGranted(AuthorizationUser $currentAuthorizationUser, string $rightName, $subject): bool
     {
-        $this->tryIncreaseRecursionCounter($rightName);
-
-        $rightExpression = $this->rightExpressions[$rightName] ?? null;
-        if ($rightExpression === null) {
-            throw new AuthorizationException(sprintf('right \'%s\' undefined', $rightName), AuthorizationException::PRIVILEGE_UNDEFINED);
+        if (in_array($rightName, $this->rightExpressionStack, true)) {
+            throw new AuthorizationException(sprintf('infinite loop caused by authorization right expression %s detected', $rightName), AuthorizationException::INFINITE_EXRPESSION_LOOP_DETECTED);
         }
+        array_push($this->rightExpressionStack, $rightName);
+
+        try {
+            $rightExpression = $this->rightExpressions[$rightName] ?? null;
+            if ($rightExpression === null) {
+                throw new AuthorizationException(sprintf('right \'%s\' undefined', $rightName), AuthorizationException::PRIVILEGE_UNDEFINED);
+            }
 
-        return $this->expressionLanguage->evaluate($rightExpression, [
-            'user' => $currentAuthorizationUser,
-            'subject' => $subject,
-        ]);
+            return $this->expressionLanguage->evaluate($rightExpression, [
+                'user' => $currentAuthorizationUser,
+                'subject' => $subject,
+            ]);
+        } finally {
+            array_pop($this->rightExpressionStack);
+        }
     }
 
     private function loadExpressions(array $expressions, array &$target): void
@@ -110,14 +132,4 @@ class AuthorizationExpressionChecker
             $target[$name] = $expression;
         }
     }
-
-    /**
-     * @throws AuthorizationException
-     */
-    private function tryIncreaseRecursionCounter(string $hint): void
-    {
-        if (++$this->callCounter > self::MAX_NUM_CALLS) {
-            throw new AuthorizationException(sprintf('possible infinite loop in authorization expression detected (%s)', $hint), AuthorizationException::INFINITE_LOOP_DETECTED);
-        }
-    }
 }

src/Authorization/EventSubscriber/AbstractGetAttributeSubscriber.php

@@ -10,8 +10,8 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 
 abstract class AbstractGetAttributeSubscriber implements EventSubscriberInterface
 {
-    /** @var GetAttributeEvent|null */
-    private $event;
+    /** @var array */
+    private $eventStack;
 
     public static function getSubscribedEvents(): array
     {
@@ -21,6 +21,11 @@ abstract class AbstractGetAttributeSubscriber implements EventSubscriberInterfac
         ];
     }
 
+    public function __construct()
+    {
+        $this->eventStack = [];
+    }
+
     public function onGetAvailableAttributes(GetAvailableAttributesEvent $event)
     {
         $event->addAttributes($this->getNewAttributes());
@@ -29,7 +34,7 @@ abstract class AbstractGetAttributeSubscriber implements EventSubscriberInterfac
     public function onGetAttributeEvent(GetAttributeEvent $event)
     {
         try {
-            $this->event = $event;
+            array_push($this->eventStack, $event);
             $attributeName = $event->getAttributeName();
 
             $event->setAttributeValue(in_array($attributeName, $this->getNewAttributes(), true) ?
@@ -37,7 +42,7 @@ abstract class AbstractGetAttributeSubscriber implements EventSubscriberInterfac
                 $this->updateExistingAttributeValue($event->getUserIdentifier(), $attributeName, $event->getAttributeValue())
             );
         } finally {
-            $this->event = null;
+            array_pop($this->eventStack);
         }
     }
 
@@ -48,7 +53,7 @@ abstract class AbstractGetAttributeSubscriber implements EventSubscriberInterfac
      */
     public function getAttribute(string $attributeName, $defaultValue = null)
     {
-        return $this->event->getAttribute($attributeName, $defaultValue);
+        return $this->eventStack[array_key_last($this->eventStack)]->getAttribute($attributeName, $defaultValue);
     }
 
     /**