security #cve-2020-5275 [Security] Fix access_control behavior with unanimous decision strategy (chalasr)

This PR was merged into the 4.4 branch.
This commit is contained in:
Nicolas Grekas 2020-03-30 13:55:16 +02:00
commit c935e4a3fb
4 changed files with 50 additions and 12 deletions

View File

@ -53,11 +53,16 @@ class AccessDecisionManager implements AccessDecisionManagerInterface
}
/**
* @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);
}

View File

@ -87,15 +87,7 @@ class AccessListener extends AbstractListener implements ListenerInterface
$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);

View File

@ -16,6 +16,7 @@ use Symfony\Component\HttpFoundation\Request;
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;
@ -227,4 +228,44 @@ class AccessListenerTest extends TestCase
$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));
}
}

View File

@ -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"