Skip to content
Permalink
Browse files Browse the repository at this point in the history
security #cve-2020-5275 [Security] Fix access_control behavior with u…
…nanimous decision strategy (chalasr)

This PR was merged into the 4.4 branch.
  • Loading branch information
nicolas-grekas committed Mar 30, 2020
2 parents 78c0bcb + 0f6a999 commit c935e4a
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 12 deletions.
Expand Up @@ -53,11 +53,16 @@ public function __construct(iterable $voters = [], string $strategy = self::STRA
}

/**
* @param bool $allowMultipleAttributes Whether to allow passing multiple values to the $attributes array
*
* {@inheritdoc}
*/
public function decide(TokenInterface $token, array $attributes, $object = null)
public function decide(TokenInterface $token, array $attributes, $object = null/*, bool $allowMultipleAttributes = false*/)
{
if (\count($attributes) > 1) {
$allowMultipleAttributes = 3 < func_num_args() && func_get_arg(3);

// Special case for AccessListener, do not remove the right side of the condition before 6.0
if (\count($attributes) > 1 && !$allowMultipleAttributes) {
@trigger_error(sprintf('Passing more than one Security attribute to "%s()" is deprecated since Symfony 4.4. Use multiple "decide()" calls or the expression language (e.g. "is_granted(...) or is_granted(...)") instead.', __METHOD__), E_USER_DEPRECATED);
}

Expand Down
10 changes: 1 addition & 9 deletions src/Symfony/Component/Security/Http/Firewall/AccessListener.php
Expand Up @@ -87,15 +87,7 @@ public function authenticate(RequestEvent $event)
$this->tokenStorage->setToken($token);
}

$granted = false;
foreach ($attributes as $key => $value) {
if ($this->accessDecisionManager->decide($token, [$key => $value], $request)) {
$granted = true;
break;
}
}

if (!$granted) {
if (!$this->accessDecisionManager->decide($token, $attributes, $request, true)) {
$exception = new AccessDeniedException();
$exception->setAttributes($attributes);
$exception->setSubject($request);
Expand Down
Expand Up @@ -16,6 +16,7 @@
use Symfony\Component\HttpKernel\Event\RequestEvent;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
use Symfony\Component\Security\Http\AccessMapInterface;
Expand Down Expand Up @@ -227,4 +228,44 @@ public function testHandleWhenTheSecurityTokenStorageHasNoToken()

$listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST));
}

public function testHandleMWithultipleAttributesShouldBeHandledAsAnd()
{
$request = new Request();

$accessMap = $this->getMockBuilder('Symfony\Component\Security\Http\AccessMapInterface')->getMock();
$accessMap
->expects($this->any())
->method('getPatterns')
->with($this->equalTo($request))
->willReturn([['foo' => 'bar', 'bar' => 'baz'], null])
;

$authenticatedToken = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock();
$authenticatedToken
->expects($this->any())
->method('isAuthenticated')
->willReturn(true)
;

$tokenStorage = new TokenStorage();
$tokenStorage->setToken($authenticatedToken);

$accessDecisionManager = $this->getMockBuilder('Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface')->getMock();
$accessDecisionManager
->expects($this->once())
->method('decide')
->with($this->equalTo($authenticatedToken), $this->equalTo(['foo' => 'bar', 'bar' => 'baz']), $this->equalTo($request), true)
->willReturn(true)
;

$listener = new AccessListener(
$tokenStorage,
$accessDecisionManager,
$accessMap,
$this->createMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface')
);

$listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST));
}
}
2 changes: 1 addition & 1 deletion src/Symfony/Component/Security/Http/composer.json
Expand Up @@ -17,7 +17,7 @@
],
"require": {
"php": "^7.1.3",
"symfony/security-core": "^4.4",
"symfony/security-core": "^4.4.7",
"symfony/http-foundation": "^3.4.40|^4.4.7|^5.0.7",
"symfony/http-kernel": "^4.4",
"symfony/property-access": "^3.4|^4.0|^5.0"
Expand Down

0 comments on commit c935e4a

Please sign in to comment.