8000 security #cve-2020-5275 [Security] Fix access_control behavior with u… · symfony/symfony@c935e4a · GitHub
[go: up one dir, main page]
Skip to content

Commit c935e4a

Browse files
nicolas-grekasnicolas-grekas
committed
security #cve-2020-5275 [Security] Fix access_control behavior with unanimous decision strategy (chalasr)
This PR was merged into the 4.4 branch.
2 parents 78c0bcb + 0f6a999 commit c935e4a

File tree

4 files changed

+50
-12
lines changed

4 files changed

+50
-12
lines changed

‎src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,16 @@ public function __construct(iterable $voters = [], string $strategy = self::STRA
5353
}
5454

5555
/**
56+
* @param bool $allowMultipleAttributes Whether to allow passing multiple values to the $attributes array
57+
*
5658
* {@inheritdoc}
5759
*/
58-
public function decide(TokenInterface $token, array $attributes, $object = null)
60+
public function decide(TokenInterface $token, array $attributes, $object = null/*, bool $allowMultipleAttributes = false*/)
5961
{
60-
if (\count($attributes) > 1) {
62+
$allowMultipleAttributes = 3 < func_num_args() && func_get_arg(3);
63+
64+
// Special case for AccessListener, do not remove the right side of the condition before 6.0
65+
if (\count($attributes) > 1 && !$allowMultipleAttributes) {
6166
@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);
6267
}
6368

‎src/Symfony/Component/Security/Http/Firewall/AccessListener.php

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,7 @@ public function authenticate(RequestEvent $event)
8787
$this->tokenStorage->setToken($token);
8888
}
8989

90-
$granted = false;
91-
foreach ($attributes as $key => $value) {
92-
if ($this->accessDecisionManager->decide($token, [$key => $value], $request)) {
93-
$granted = true;
94-
break;
95-
}
96-
}
97-
98-
if (!$granted) {
90+
if (!$this->accessDecisionManager->decide($token, $attributes, $request, true)) {
9991
$exception = new AccessDeniedException();
10092
$exception->setAttributes($attributes);
10193
$exception->setSubject($request);

‎src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\HttpKernel\Event\RequestEvent;
1717
use Symfony\Component\HttpKernel\HttpKernelInterface;
1818
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
19+
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
1920
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
2021
use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
2122
use Symfony\Component\Security\Http\AccessMapInterface;
@@ -227,4 +228,44 @@ public function testHandleWhenTheSecurityTokenStorageHasNoToken()
227228

228229
$listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST));
229230
}
231+
232+
public function testHandleMWithultipleAttributesShouldBeHandledAsAnd()
233+
{
234+
$request = new Request();
235+
236+
$accessMap = $this->getMockBuilder('Symfony\Component\Security\Http\AccessMapInterface')->getMock();
237+
$accessMap
238+
->expects($this->any())
239+
->method('getPatterns')
240+
->with($this->equalTo($request))
241+
->willReturn([['foo' => 'bar', 'bar' => 'baz'], null])
242+
;
243+
244+
$authenticatedToken = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock();
245+
$authenticatedToken
246+
->expects($this->any())
247+
->method('isAuthenticated')
248+
->willReturn(true)
249+
;
250+
251+
$tokenStorage = new TokenStorage();
252+
$tokenStorage->setToken($authenticatedToken);
253+
254+
$accessDecisionManager = $this->getMockBuilder('Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface')->getMock();
255+
$accessDecisionManager
256+
->expects($this->once())
257+
->method('decide')
258+
->with($this->equalTo($authenticatedToken), $this->equalTo(['foo' => 'bar', 'bar' => 'baz']), $this->equalTo($request), true)
259+
->willReturn(true)
260+
;
261+
262+
$listener = new AccessListener(
263+
$tokenStorage,
264+
$accessDecisionManager,
265+
$accessMap,
266+
$this->createMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface')
267+
);
268+
269+
$listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST));
270+
}
230271
}

‎src/Symfony/Component/Security/Http/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
],
1818
"require": {
1919
"php": "^7.1.3",
20-
"symfony/security-core": "^4.4",
20+
"symfony/security-core": "^4.4.7",
2121
"symfony/http-foundation": "^3.4.40|^4.4.7|^5.0.7",
2222
"symfony/http-kernel": "^4.4",
2323
"symfony/property-access": "^3.4|^4.0|^5.0"

0 commit comments

Comments
 (0)
0