8000 [Property Accessor] Error on reserved characters from CacheAdapter · Issue #29329 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

[Property Accessor] Error on reserved characters from CacheAdapter #29329

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
shavounet opened this issue Nov 26, 2018 · 3 comments
Closed

[Property Accessor] Error on reserved characters from CacheAdapter #29329

shavounet opened this issue Nov 26, 2018 · 3 comments
Labels
Bug Cache PropertyAccess Status: Needs Review

Comments

@shavounet
Copy link

Bug description: when using the built-in PropertyAccessor service, accessing an array property containing / in its key throws an error.

Example

  • in a standalone bin/test.php file, in a working Symfony instance
<?php
$loader = require __DIR__.'/../vendor/autoload.php';

$kernel = new AppKernel('dev', true);
$kernel->boot();

var_dump($kernel->getContainer()->get('property_accessor')->getValue(['a/b' => 'ok'], '[a/b]'));
  • execution and stack
$ php bin/test.php
PHP Fatal error:  Uncaught Symfony\Component\Cache\Exception\InvalidArgumentException: Cache key "p[a/b]" contains reserved characters {}()/\@: in /var/www/current/vendor/symfony/symfony/src/Symfony/Component/Cache/CacheItem.php:162
Stack trace:
#0 /var/www/current/vendor/symfony/symfony/src/Symfony/Component/Cache/Traits/ArrayTrait.php(45): Symfony\Component\Cache\CacheItem::validateKey('p[a/b]')
#1 /var/www/current/vendor/symfony/symfony/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php(56): Symfony\Component\Cache\Adapter\ArrayAdapter->hasItem('p[a/b]')
#2 /var/www/current/vendor/symfony/symfony/src/Symfony/Component/PropertyAccess/PropertyAccessor.php(875): Symfony\Component\Cache\Adapter\ArrayAdapter->getItem('p[a/b]')
#3 /var/www/current/vendor/symfony/symfony/src/Symfony/Component/PropertyAccess/PropertyAccessor.php(160): Symfony\Component\PropertyAccess\PropertyAccessor->getPropertyPath('[a/b]')
#4 /var/www/current/bin/test.php(8): Symfony\Component\PropertyAccess\PropertyAccessor->getValue(Array, '[a/b]')
#5 {main} in /var/www/current/vendor/symfony/symfony/src/Symfony/Component/Cache/CacheItem.php on line 162

Analysis
The PropertyAccessor component uses a standard PSR6 cache to increase it's performance, but the key is validated by Symfony\Component\Cache\CacheItem::validateKey, and some characters are forbidden. The problem is that any of those characters are allowed as an array key (source). It means some properties can never be accessed through the property accessor.

As a quick workaround, we will disable the cache.

Side note: an empty array key is also not reachable (even if it's certainly a bad practice...)

Resolution proposals

  • Hash the key (but should it be protected against collisions with a mapping ?)
  • Catch the cache component error, add a warning and skip caching for those property (not ideal, but a quick and safe solution...)
  • Find a way to escape those characters

Context

  • Symfony 3.4
  • PHP 7.2
@nicolas-grekas
Copy link
Member

Thanks for reporting.
Closing as duplicate of #29293, fixed in #29313.

@shavounet
Copy link
Author

Wow that was quick :-)

@nicolas-grekas I'm looking at your fix, and I think there might be an issue... You're escaping the key only on character detection, not always, which implies that in an extremely rare circumstance (or as an exploit ?) we can have a cache collision...

$obj = new stdClass();
$obj->{"a/b"} = '1';
$obj->{"a%2Fb"} = '2';

var_dump($kernel->getContainer()->get('property_accessor')->getValue($obj, "a/b"));
var_dump($kernel->getContainer()->get('property_accessor')->getValue($obj, "a%2Fb"));

displays

/var/www/current/bin/test.php:12:
string(1) "1"
/var/www/current/bin/test.php:13:
string(1) "1"

instead of "1", then "2"

@nicolas-grekas nicolas-grekas mentioned this issue Nov 26, 2018
Merged
@nicolas-grekas
Copy link
Member

Agreed, fixed in #29332

nicolas-grekas added a commit that referenced this issue Nov 26, 2018
@nicolas-grekas
bug #29332 [PropertyAccess] make cache keys encoding bijective (nicol…
1f59fa4 
…as-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[PropertyAccess] make cache keys encoding bijective

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29329 (comment)
| License       | MIT
| Doc PR        | -

Commits
-------

4fe566b [PropertyAccess] make cache keys encoding bijective
@ndench ndench mentioned this issue Apr 27, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Cache PropertyAccess Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@nicolas-grekas @xabbuh @shavounet @carsonbot
0