Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org). Thia is a

## TBD - 5.8.0

### Security Note

- File::prohibitWrappers and Drawing::setPath now reject phar paths with extra leading slashes (e.g. phar:///…) that escaped the prior parse_url-based filter. No security exploit was possible even with the extra slashes. [PR #4876](https://github.com/PHPOffice/PhpSpreadsheet/pull/4876)

### Added

- Optional method to increase Calculation Engine's parsing speed. [PR #4829](https://github.com/PHPOffice/PhpSpreadsheet/pull/4829)
Expand Down
15 changes: 9 additions & 6 deletions src/PhpSpreadsheet/Shared/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,19 +134,22 @@ public static function temporaryFilename(): string
}

/**
* All filenames starting with protocol (e.g. phar://) are prohibited.
* Blocks phar:// and similar RCE-bearing wrappers.
* Note that many protocols, including http and zip, will already
* return false for is_file.
* A whitelist of protocols may be added if needed in future.
* data: is intentionally allowed (see #4823); callers needing strict
* on-disk-only semantics must validate $filename themselves.
*/
public static function prohibitWrappers(string $filename): void
{
$scheme = parse_url($filename, PHP_URL_SCHEME);
// strlen check > 1 to avoid issues with Windows absolute paths (e.g. C:\...), Windows quirks :)
// since no built-in or commonly registered PHP stream wrapper uses a single-character scheme, this should be ok, to my knowledge
if (is_string($scheme) && strlen($scheme) > 1) {
if (
preg_match('~^phar://~i', $filename)
|| (preg_match('/^([\w\s\x00-\x1f]+):/u', $filename) && !preg_match('/^([\w]+):/u', $filename))
|| preg_match('~^php://.*phar:~i', $filename)
) {
throw new Exception(
"Stream wrappers are not permitted as file paths: {$filename}"
"Disallowed stream wrapper used for {$filename}"
);
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/PhpSpreadsheet/Worksheet/Drawing.php
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,11 @@ public function setPath(string $path, bool $verifyFile = true, ?ZipArchive $zip
}
}
// Check if a URL has been passed. https://stackoverflow.com/a/2058596/1252979
} elseif (filter_var($path, FILTER_VALIDATE_URL) || (preg_match('/^([\w\s\x00-\x1f]+):/u', $path) && !preg_match('/^([\w]+):/u', $path))) {
} elseif (
filter_var($path, FILTER_VALIDATE_URL)
|| preg_match('~^phar://~i', $path)
|| (preg_match('/^([\w\s\x00-\x1f]+):/u', $path) && !preg_match('/^([\w]+):/u', $path))
) {
if (!Preg::isMatch('/^(http|https|file|ftp|s3):/', $path)) {
throw new PhpSpreadsheetException('Invalid protocol for linked drawing');
}
Expand Down
3 changes: 3 additions & 0 deletions tests/PhpSpreadsheetTests/Reader/Html/HtmlImage2Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,10 @@ public static function providerBadProtocol(): array
'mailto' => ['mailto:xyz@example.com'],
'mailto whitespace' => ['mail to:xyz@example.com'],
'phar' => ['phar://example.com/image.phar'],
'phar mixed case' => ['Phar://example.com/image.phar'],
'phar with 3 slashes' => ['phar:///example.com/image.phar'],
'phar control' => ["\x14phar://example.com/image.phar"],
'filter with phar' => ['php://filter/read=convert.base64-encode/resource=phar:///tmp/x.Phar'],
];
}
}
26 changes: 25 additions & 1 deletion tests/PhpSpreadsheetTests/Reader/NoPharTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,35 @@ class NoPharTest extends TestCase
public function testNoPhar(string $reader): void
{
$this->expectException(SpreadsheetException::class);
$this->expectExceptionMessage('Stream wrappers are not permitted');
$this->expectExceptionMessage('Disallowed stream wrapper');
$reader = new $reader();
$reader->load('phar://anyoldname');
}

/**
* @param class-string<IReader> $reader
*/
#[DataProvider('providerReaders')]
public function testPhar3Slashes(string $reader): void
{
$invalidProtocol = [
'3 slashes' => 'phar:///anyoldname',
'mixed case' => 'Phar:///anyoldname',
'embedded space' => 'ph ar://anyoldname',
'embedded control character' => "ph\x04ar://anyoldname",
'filter with phar' => 'php://filter/read=convert.base64-encode/resource=phar:///tmp/x.Phar',
];
$reader = new $reader();
foreach ($invalidProtocol as $key => $value) {
try {
$reader->load($value);
self::fail("Should have thrown exception - $key");
} catch (SpreadsheetException $e) {
self::assertStringContainsString('Disallowed stream wrapper', $e->getMessage(), $key);
}
}
}

/**
* @return array<array<class-string<IReader>>>
*/
Expand Down
Loading