Prevent Loop in Shared/File::realpath (#3809)

Fix #3807. Function attempts to rationalize `..` in filenames in a way that normally works just fine. Reporter notes that at least one of the filenames that will be analyzed when a spreadsheet is read can be maliciously altered in a manner which does not harm Excel when reading the file, but which puts PhpSpreadsheet into a loop. This PR fixes the problem.
This commit is contained in:
oleibman
2023-12-04 16:58:53 -08:00
committed by GitHub
parent 2b528681d6
commit df3c6d9122
3 changed files with 39 additions and 3 deletions
+3 -3
View File
@@ -94,9 +94,9 @@ class File
$pathArray = explode('/', $filename);
while (in_array('..', $pathArray) && $pathArray[0] != '..') {
$iMax = count($pathArray);
for ($i = 0; $i < $iMax; ++$i) {
if ($pathArray[$i] == '..' && $i > 0) {
unset($pathArray[$i], $pathArray[$i - 1]);
for ($i = 1; $i < $iMax; ++$i) {
if ($pathArray[$i] == '..') {
array_splice($pathArray, $i - 1, 2);
break;
}
@@ -0,0 +1,36 @@
<?php
declare(strict_types=1);
namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
class Issue3807Test extends \PHPUnit\Framework\TestCase
{
private static string $testbook = 'tests/data/Reader/XLSX/issue.3807.xlsx';
public function testPreliminaries(): void
{
$file = 'zip://';
$file .= self::$testbook;
$file .= '#xl/_rels/workbook.xml.rels';
$data = file_get_contents($file);
// rels file points to non-existent theme file
if ($data === false) {
self::fail('Unable to read file');
} else {
self::assertStringContainsString('Target="theme/theme1.xml/../../../../../../../../1.txt"', $data);
}
}
public function testLoadable(): void
{
$reader = new Xlsx();
$spreadsheet = $reader->load(self::$testbook);
$sheet = $spreadsheet->getActiveSheet();
// Assert anything to confirm read succeeded
self::assertSame(1, $sheet->getCell('B1')->getValue());
$spreadsheet->disconnectWorksheets();
}
}
Binary file not shown.