mirror/php-phpbb
1
0
Fork 0
mirror of https://github.com/phpbb/phpbb.git synced 2025-06-07 15:05:43 +02:00
Code Issues Releases Wiki Activity

[ticket/17301] Ensure reading invalid cache files is aborted

Browse Source 
PHPBB-17301
This commit is contained in:
Marc Alexander 2024-06-11 20:12:01 +02:00
parent cd0e682984
commit f98f2c5896
No known key found for this signature in database
GPG Key ID: 50E0D2423696F995
2 changed files with 170 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
phpBB/phpbb/cache/driver/file.php vendored
View File

@ -330,6 +330,27 @@ class file extends \phpbb\cache\driver\base
return $query_result;
}
/**
* Cleanup when loading invalid data global file
*
* @param string $file Filename
* @param resource $handle
*
* @return void
*/
private function cleanup_invalid_data_global(string $file, $handle): void
{
if (is_resource($handle))
{
fclose($handle);
}
$this->vars = $this->var_expires = [];
$this->is_modified = false;
$this->remove_file($file);
}
/**
* Read cached data from a specified file
*
@ -372,14 +393,7 @@ class file extends \phpbb\cache\driver\base
if (!is_numeric($bytes) || ($bytes = (int) $bytes) === 0)
{
// We cannot process the file without a valid number of bytes
// so we discard it
fclose($handle);
$this->vars = $this->var_expires = array();
$this->is_modified = false;
$this->remove_file($file);
$this->cleanup_invalid_data_global($file, $handle);
return false;
}
@ -392,9 +406,17 @@ class file extends \phpbb\cache\driver\base
}
$var_name = substr(fgets($handle), 0, -1);
$data_length = $bytes - strlen($var_name);
if ($data_length <= 0)
{
$this->cleanup_invalid_data_global($file, $handle);
return false;
}
// Read the length of bytes that consists of data.
$data = fread($handle, $bytes - strlen($var_name));
$data = fread($handle, $data_length);
$data = @unserialize($data);
// Don't use the data if it was invalid

140
tests/cache/file_driver_test.php vendored
View File

@ -17,6 +17,9 @@ class phpbb_cache_file_driver_test extends phpbb_cache_common_test_case
{
private $cache_dir;
/** @var \phpbb\cache\driver\file */
private $cache_file;
public function getDataSet()
{
return $this->createXMLDataSet(__DIR__ . '/fixtures/config.xml');
@ -36,7 +39,8 @@ class phpbb_cache_file_driver_test extends phpbb_cache_common_test_case
}
$this->create_cache_dir();
$this->driver = new \phpbb\cache\driver\file($this->cache_dir);
$this->cache_file = new \phpbb\cache\driver\file($this->cache_dir);
$this->driver = $this->cache_file;
}
protected function tearDown(): void
@ -49,6 +53,140 @@ class phpbb_cache_file_driver_test extends phpbb_cache_common_test_case
parent::tearDown();
}
public function test_read_not_readable()
{
global $phpEx;
// Create file that is not readable
$this->assertTrue($this->cache_file->_write('unreadable', 'foo', time() + 86400));
$filename = "{$this->cache_dir}unreadable.$phpEx";
@chmod($filename, 0000);
$this->assertFalse($this->cache_file->_read('unreadable'));
@chmod($filename, 0600);
$this->assertNotFalse($this->cache_file->_read('unreadable'));
}
public function test_read_data_global_invalid()
{
global $phpEx;
$reflectionCacheVars = new \ReflectionProperty($this->cache_file, 'vars');
$reflectionCacheVars->setAccessible(true);
$reflectionCacheVars->setValue($this->cache_file, ['foo' => 'bar']);
$reflectionCacheVarExpires = new \ReflectionProperty($this->cache_file, 'var_expires');
$reflectionCacheVarExpires->setAccessible(true);
$reflectionCacheVarExpires->setValue($this->cache_file, ['foo' => time() + 86400]);
// Create file in invalid format
$this->assertTrue($this->cache_file->_write('data_global'));
$filename = "{$this->cache_dir}data_global.$phpEx";
$cache_data = file_get_contents($filename);
// Force negative read when retrieving data_global
$cache_data = str_replace("\n13\n", "\n1\n", $cache_data);
file_put_contents($filename, $cache_data);
$this->assertFalse($this->cache_file->_read('data_global'));
}
public function test_read_data_global_zero_bytes()
{
global $phpEx;
$reflectionCacheVars = new \ReflectionProperty($this->cache_file, 'vars');
$reflectionCacheVars->setAccessible(true);
$reflectionCacheVars->setValue($this->cache_file, ['foo' => 'bar']);
$reflectionCacheVarExpires = new \ReflectionProperty($this->cache_file, 'var_expires');
$reflectionCacheVarExpires->setAccessible(true);
$reflectionCacheVarExpires->setValue($this->cache_file, ['foo' => time() + 86400]);
// Create file in invalid format
$this->assertTrue($this->cache_file->_write('data_global'));
$filename = "{$this->cache_dir}data_global.$phpEx";
$cache_data = file_get_contents($filename);
// Force negative read when retrieving data_global
$cache_data = str_replace("\n13\n", "\n0\n", $cache_data);
file_put_contents($filename, $cache_data);
$this->assertFalse($this->cache_file->_read('data_global'));
}
public function test_read_data_global_hex_bytes()
{
global $phpEx;
$reflectionCacheVars = new \ReflectionProperty($this->cache_file, 'vars');
$reflectionCacheVars->setAccessible(true);
$reflectionCacheVars->setValue($this->cache_file, ['foo' => 'bar']);
$reflectionCacheVarExpires = new \ReflectionProperty($this->cache_file, 'var_expires');
$reflectionCacheVarExpires->setAccessible(true);
$reflectionCacheVarExpires->setValue($this->cache_file, ['foo' => time() + 86400]);
// Create file in invalid format
$this->assertTrue($this->cache_file->_write('data_global'));
$filename = "{$this->cache_dir}data_global.$phpEx";
$cache_data = file_get_contents($filename);
// Force negative read when retrieving data_global
$cache_data = str_replace("\n13\n", "\nA\n", $cache_data);
file_put_contents($filename, $cache_data);
$this->assertFalse($this->cache_file->_read('data_global'));
}
public function test_read_data_global_expired()
{
$reflectionCacheVars = new \ReflectionProperty($this->cache_file, 'vars');
$reflectionCacheVars->setAccessible(true);
$reflectionCacheVars->setValue($this->cache_file, ['foo' => 'bar']);
$reflectionCacheVarExpires = new \ReflectionProperty($this->cache_file, 'var_expires');
$reflectionCacheVarExpires->setAccessible(true);
$reflectionCacheVarExpires->setValue($this->cache_file, ['foo' => time() - 86400]);
// Create file in invalid format
$this->assertTrue($this->cache_file->_write('data_global'));
// Clear data
$reflectionCacheVars->setValue($this->cache_file, []);
$reflectionCacheVarExpires->setValue($this->cache_file, []);
$this->assertTrue($this->cache_file->_read('data_global'));
// Check data, should be empty
$this->assertEquals([], $reflectionCacheVars->getValue($this->cache_file));
}
public function test_read_data_global()
{
$reflectionCacheVars = new \ReflectionProperty($this->cache_file, 'vars');
$reflectionCacheVars->setAccessible(true);
$expectedVars = ['foo' => 'bar'];
$reflectionCacheVars->setValue($this->cache_file, $expectedVars);
$reflectionCacheVarExpires = new \ReflectionProperty($this->cache_file, 'var_expires');
$reflectionCacheVarExpires->setAccessible(true);
$expectedVarExpires = ['foo' => time() + 86400];
$reflectionCacheVarExpires->setValue($this->cache_file, $expectedVarExpires);
// Create file in invalid format
$this->assertTrue($this->cache_file->_write('data_global'));
// Clear data
$reflectionCacheVars->setValue($this->cache_file, []);
$reflectionCacheVarExpires->setValue($this->cache_file, []);
$this->assertEquals([], $reflectionCacheVars->getValue($this->cache_file));
$this->assertEquals([], $reflectionCacheVarExpires->getValue($this->cache_file));
$this->assertTrue($this->cache_file->_read('data_global'));
// Check data, should be empty
$this->assertEquals($expectedVars, $reflectionCacheVars->getValue($this->cache_file));
$this->assertEquals($expectedVarExpires, $reflectionCacheVarExpires->getValue($this->cache_file));
}
private function create_cache_dir()
{
$this->get_test_case_helpers()->makedirs($this->cache_dir);