bug #20734 [Security] AbstractVoter->supportsAttribute gives false positive if attribute is zero (0) (martynas-foodpanda)

This PR was merged into the 2.7 branch.

Discussion
----------

[Security] AbstractVoter->supportsAttribute gives false positive if attribute is zero (0)

| Q             | A
| ------------- | ---
| Branch?       |  2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Issue is easy to reproduce with test giving negative data set.
0 should not pass as supported attribute for any set of attributes but it does as in_array in the method does not use flag 'strict' set to true.

As this is abstract voter and is used by users with their code flag 'strict' should be set to true.
Since is there in 2.7 and 2.8 (LTS) IMHO it should be fixed.

Commits
-------

8306530 [Security] AbstractVoter method supportsAttribute gives false positive if attribute is zero (0)
This commit is contained in:
Fabien Potencier 2016-12-14 09:11:55 +01:00
commit 482e9edc50
2 changed files with 72 additions and 1 deletions

View File

@ -26,7 +26,7 @@ abstract class AbstractVoter implements VoterInterface
*/
public function supportsAttribute($attribute)
{
return in_array($attribute, $this->getSupportedAttributes());
return in_array($attribute, $this->getSupportedAttributes(), true);
}
/**

View File

@ -16,6 +16,9 @@ use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
class AbstractVoterTest extends \PHPUnit_Framework_TestCase
{
/**
* @var TokenInterface
*/
protected $token;
protected function setUp()
@ -23,6 +26,9 @@ class AbstractVoterTest extends \PHPUnit_Framework_TestCase
$this->token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');
}
/**
* @return array
*/
public function getTests()
{
return array(
@ -53,6 +59,71 @@ class AbstractVoterTest extends \PHPUnit_Framework_TestCase
$this->assertEquals($expectedVote, $voter->vote($this->token, $object, $attributes), $message);
}
/**
* @return array
*/
public function getSupportsAttributeData()
{
return array(
'positive_string_edit' => array(
'expected' => true,
'attribute' => 'EDIT',
'message' => 'expected TRUE given as attribute EDIT is supported',
),
'positive_string_create' => array(
'expected' => true,
'attribute' => 'CREATE',
'message' => 'expected TRUE as given attribute CREATE is supported',
),
'negative_string_read' => array(
'expected' => false,
'attribute' => 'READ',
'message' => 'expected FALSE as given attribute READ is not supported',
),
'negative_string_random' => array(
'expected' => false,
'attribute' => 'random',
'message' => 'expected FALSE as given attribute "random" is not supported',
),
'negative_string_0' => array(
'expected' => false,
'attribute' => '0',
'message' => 'expected FALSE as given attribute "0" is not supported',
),
// this set of data gives false positive if in_array is not used with strict flag set to 'true'
'negative_int_0' => array(
'expected' => false,
'attribute' => 0,
'message' => 'expected FALSE as given attribute 0 is not string',
),
'negative_int_1' => array(
'expected' => false,
'attribute' => 1,
'message' => 'expected FALSE as given attribute 1 is not string',
),
'negative_int_7' => array(
'expected' => false,
'attribute' => 7,
'message' => 'expected FALSE as attribute 7 is not string',
),
);
}
/**
* @dataProvider getSupportsAttributeData
*
* @param bool $expected
* @param string $attribute
* @param string $message
*/
public function testSupportsAttribute($expected, $attribute, $message)
{
$voter = new AbstractVoterTest_Voter();
$this->assertEquals($expected, $voter->supportsAttribute($attribute), $message);
}
}
class AbstractVoterTest_Voter extends AbstractVoter