Security Patches

src/PhpSpreadsheet/Helper/Downloader.php

@@ -29,18 +29,16 @@ class Downloader
 
     public function __construct(string $folder, string $filename, ?string $filetype = null)
     {
-        if ((is_dir($folder) === false) || (is_readable($folder) === false)) {
-            throw new Exception('Folder is not accessible');
-        }
-        $filepath = "{$folder}/{$filename}";
-        $this->filepath = (string) realpath($filepath);
-        $this->filename = basename($filepath);
-        if ((file_exists($this->filepath) === false) || (is_readable($this->filepath) === false)) {
+        clearstatcache();
+        $filepath = realpath("{$folder}/{$filename}");
+        if ($filepath === false || !is_file($filepath) || !is_readable($filepath)) {
             throw new Exception('File not found, or cannot be read');
         }
+        $this->filepath = $filepath;
+        $this->filename = basename($this->filepath);
 
-        $filetype ??= pathinfo($filename, PATHINFO_EXTENSION);
-        if (array_key_exists(strtolower($filetype), self::CONTENT_TYPES) === false) {
+        $filetype ??= pathinfo($this->filename, PATHINFO_EXTENSION);
+        if (!array_key_exists(strtolower($filetype), self::CONTENT_TYPES)) {
             throw new Exception('Invalid filetype: cannot be downloaded');
         }
         $this->filetype = strtolower($filetype);

src/PhpSpreadsheet/IOFactory.php

@@ -233,4 +233,16 @@ abstract class IOFactory
 
         self::$readers[$readerType] = $readerClass;
     }
+
+    /**
+     * @return array<string, class-string<IReader>>
+     *
+     * @internal
+     *
+     * @codeCoverageIgnore
+     */
+    public static function getReaders(): array
+    {
+        return self::$readers;
+    }
 }

src/PhpSpreadsheet/Reader/Xml.php

@@ -78,6 +78,7 @@ class Xml extends BaseReader
         ];
 
         // Open file
+        File::assertFile($filename);
         $data = (string) file_get_contents($filename);
         $data = $this->getSecurityScannerOrThrow()->scan($data);
 

src/PhpSpreadsheet/Shared/File.php

@@ -138,11 +138,30 @@ class File
         return $filename;
     }
 
+    /**
+     * All filenames starting with protocol (e.g. phar://) are prohibited.
+     * 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.
+     */
+    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) {
+            throw new Exception(
+                "Stream wrappers are not permitted as file paths: {$filename}"
+            );
+        }
+    }
+
     /**
      * Assert that given path is an existing file and is readable, otherwise throw exception.
      */
     public static function assertFile(string $filename, string $zipMember = ''): void
     {
+        self::prohibitWrappers($filename);
         if (!is_file($filename)) {
             throw new ReaderException('File "' . $filename . '" does not exist.');
         }
@@ -165,9 +184,11 @@ class File
 
     /**
      * Same as assertFile, except return true/false and don't throw Exception.
+     * Will nevertheless throw if filename uses invalid protocol, e.g. phar.
      */
     public static function testFileNoThrow(string $filename, ?string $zipMember = null): bool
     {
+        self::prohibitWrappers($filename);
         if (!is_file($filename)) {
             return false;
         }

src/PhpSpreadsheet/Style/NumberFormat/Formatter.php

@@ -14,6 +14,7 @@ class Formatter extends BaseFormatter
      * Matches any @ symbol that isn't enclosed in quotes.
      */
     private const SYMBOL_AT = '/@(?=(?:[^"]*"[^"]*")*[^"]*\Z)/miu';
+    private const QUOTE_REPLACEMENT = "\u{fffe}"; // invalid Unicode character
 
     /**
      * Matches any ; symbol that isn't enclosed in quotes, for a "section" split.
@@ -122,8 +123,28 @@ class Formatter extends BaseFormatter
         }
         // For now we do not treat strings in sections, although section 4 of a format code affects strings
         // Process a single block format code containing @ for text substitution
-        if (preg_match(self::SECTION_SPLIT, $format) === 0 && preg_match(self::SYMBOL_AT, $format) === 1) {
-            return str_replace('"', '', preg_replace(self::SYMBOL_AT, (string) $value, $format) ?? '');
+        $formatx = str_replace('\"', self::QUOTE_REPLACEMENT, $format);
+        if (preg_match(self::SECTION_SPLIT, $format) === 0 && preg_match(self::SYMBOL_AT, $formatx) === 1) {
+            if (!str_contains($format, '"')) {
+                return str_replace('@', "$value", $format);
+            }
+            //escape any dollar signs on the string, so they are not replaced with an empty value
+            $value = str_replace(
+                ['$', '"'],
+                ['\$', self::QUOTE_REPLACEMENT],
+                (string) $value
+            );
+            $temp = preg_replace(self::SYMBOL_AT, $value, $formatx) ?? $value;
+            if (is_callable($callBack)) {
+                $temp = $callBack($temp, $formatx);
+            }
+            /** @var string $temp */
+
+            return str_replace(
+                ['"', self::QUOTE_REPLACEMENT],
+                ['', '"'],
+                $temp
+            );
         }
 
         // If we have a text value, return it "as is"

src/PhpSpreadsheet/Writer/Html.php

@@ -1681,7 +1681,7 @@ class Html extends BaseWriter
         }
 
         // convert to PCDATA
-        $result = htmlspecialchars($value, Settings::htmlEntityFlags());
+        $result = htmlspecialchars($value, ENT_NOQUOTES);
 
         // color span tag
         if ($color !== null) {

tests/PhpSpreadsheetTests/Reader/NoPharTest.php

@@ -0,0 +1,40 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader;
+
+use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
+use PhpOffice\PhpSpreadsheet\IOFactory;
+use PhpOffice\PhpSpreadsheet\Reader\IReader;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+class NoPharTest extends TestCase
+{
+    /**
+     * @param class-string<IReader> $reader
+     */
+    #[DataProvider('providerReaders')]
+    public function testNoPhar(string $reader): void
+    {
+        $this->expectException(SpreadsheetException::class);
+        $this->expectExceptionMessage('Stream wrappers are not permitted');
+        $reader = new $reader();
+        $reader->load('phar://anyoldname');
+    }
+
+    /**
+     * @return array<array<class-string<IReader>>>
+     */
+    public static function providerReaders(): array
+    {
+        $readers = IOFactory::getReaders();
+        $array = [];
+        foreach ($readers as $key => $reader) {
+            $array[$key] = [$reader];
+        }
+
+        return $array;
+    }
+}

tests/PhpSpreadsheetTests/Writer/Html/AtSignFormatTest.php

@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Writer\Html;
+
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheet\Writer\Html as HtmlWriter;
+use PHPUnit\Framework\TestCase;
+
+class AtSignFormatTest extends TestCase
+{
+    public function testAtSignFormat(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $payload = '<img src=x onerror=alert(document.domain)>';
+        $formatCode = '@ "items"';
+        $sheet->setCellValue('A1', $payload);
+        $sheet->getStyle('A1')
+            ->getNumberFormat()
+            ->setFormatCode($formatCode);
+
+        $writer = new HtmlWriter($spreadsheet);
+        $html = $writer->generateHTMLAll();
+        self::assertStringContainsString('<td class="column0 style1 s">&lt;img src=x onerror=alert(document.domain)&gt; items</td>', $html);
+        $spreadsheet->disconnectWorksheets();
+    }
+}