tedious / Stash

The place to keep your cache.
http://www.stashphp.com
BSD 3-Clause "New" or "Revised" License
960 stars 133 forks source link

Filesystem Driver: Fix dir split not finishing cleanly #405

Closed the-eater closed 2 years ago

the-eater commented 2 years ago

I was using Stash as the cache backend for Doctrine, and, being nosy replaced the key hash function with a transparent function (fn($data) => $data) and noticed that the names of the files are not completely finished

Screenshot showing the path "ca/ch/stash/_defaul"

which was a bit alarming because this can cause unintentional cache conflicts or poisoning*

After digging around I found

https://github.com/tedious/Stash/blob/1a9c7557f928ffa00168fe607725d6cb4e4763da/src/Stash/Driver/FileSystem.php#L292-L298

the $i == $this->directorySplit was supposed to prevent this, however this will always be false, thus I have now replaced it with a bit more robust way of splitting that should prevent this.

and which now works correctly as shown here

Screenshot showing the path "ca/che/stash/_default"

*: So this can actually open to cache poisoning attacks, but since the default uses md5 which has an even amount of bytes this is a non-issue unless people use key hash functions with an uneven length return, but these are scarce so ¯\_(ツ)_/¯, an additional "is the key as stored in the item the same as requested" check might be a nice to have, and could prevent this

PS: I am on PHP 8.1 and can't run the outdated phpunit (ok ran my new check by hotfixing it)