Skip to content

Commit df3c6d9

Browse files
authored
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.
1 parent 2b52868 commit df3c6d9

File tree

3 files changed

+39
-3
lines changed

3 files changed

+39
-3
lines changed

src/PhpSpreadsheet/Shared/File.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,9 @@ public static function realpath(string $filename): string
9494
$pathArray = explode('/', $filename);
9595
while (in_array('..', $pathArray) && $pathArray[0] != '..') {
9696
$iMax = count($pathArray);
97-
for ($i = 0; $i < $iMax; ++$i) {
98-
if ($pathArray[$i] == '..' && $i > 0) {
99-
unset($pathArray[$i], $pathArray[$i - 1]);
97+
for ($i = 1; $i < $iMax; ++$i) {
98+
if ($pathArray[$i] == '..') {
99+
array_splice($pathArray, $i - 1, 2);
100100

101101
break;
102102
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
6+
7+
use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
8+
9+
class Issue3807Test extends \PHPUnit\Framework\TestCase
10+
{
11+
private static string $testbook = 'tests/data/Reader/XLSX/issue.3807.xlsx';
12+
13+
public function testPreliminaries(): void
14+
{
15+
$file = 'zip://';
16+
$file .= self::$testbook;
17+
$file .= '#xl/_rels/workbook.xml.rels';
18+
$data = file_get_contents($file);
19+
// rels file points to non-existent theme file
20+
if ($data === false) {
21+
self::fail('Unable to read file');
22+
} else {
23+
self::assertStringContainsString('Target="theme/theme1.xml/../../../../../../../../1.txt"', $data);
24+
}
25+
}
26+
27+
public function testLoadable(): void
28+
{
29+
$reader = new Xlsx();
30+
$spreadsheet = $reader->load(self::$testbook);
31+
$sheet = $spreadsheet->getActiveSheet();
32+
// Assert anything to confirm read succeeded
33+
self::assertSame(1, $sheet->getCell('B1')->getValue());
34+
$spreadsheet->disconnectWorksheets();
35+
}
36+
}
8.74 KB
Binary file not shown.

0 commit comments

Comments
 (0)