Backport Security Patch

src/PhpSpreadsheet/Reader/Xlsx.php

@@ -1310,7 +1310,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                                         $hfImages[$shapeId]->setName((string) $imageData['title']);
                                                     }
 
-                                                    $hfImages[$shapeId]->setPath('zip://' . File::realpath($filename) . '#' . $drawings[(string) $imageData['relid']], false);
+                                                    $hfImages[$shapeId]->setPath('zip://' . File::realpath($filename) . '#' . $drawings[(string) $imageData['relid']], false, $zip);
                                                     $hfImages[$shapeId]->setResizeProportional(false);
                                                     $hfImages[$shapeId]->setWidth($style['width']);
                                                     $hfImages[$shapeId]->setHeight($style['height']);
@@ -1422,7 +1422,8 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                                         $objDrawing->setPath(
                                                             'zip://' . File::realpath($filename) . '#'
                                                             . $images[$embedImageKey],
-                                                            false
+                                                            false,
+                                                            $zip
                                                         );
                                                     } else {
                                                         $linkImageKey = (string) self::getArrayItem(
@@ -1433,6 +1434,9 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                                             $url = str_replace('xl/drawings/', '', $images[$linkImageKey]);
                                                             $objDrawing->setPath($url);
                                                         }
+                                                        if ($objDrawing->getPath() === '') {
+                                                            continue;
+                                                        }
                                                     }
                                                     $objDrawing->setCoordinates(Coordinate::stringFromColumnIndex(((int) $oneCellAnchor->from->col) + 1) . ($oneCellAnchor->from->row + 1));
 
@@ -1511,7 +1515,8 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                                         $objDrawing->setPath(
                                                             'zip://' . File::realpath($filename) . '#'
                                                             . $images[$embedImageKey],
-                                                            false
+                                                            false,
+                                                            $zip
                                                         );
                                                     } else {
                                                         $linkImageKey = (string) self::getArrayItem(
@@ -1522,6 +1527,9 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                                             $url = str_replace('xl/drawings/', '', $images[$linkImageKey]);
                                                             $objDrawing->setPath($url);
                                                         }
+                                                        if ($objDrawing->getPath() === '') {
+                                                            continue;
+                                                        }
                                                     }
                                                     $objDrawing->setCoordinates(Coordinate::stringFromColumnIndex(((int) $twoCellAnchor->from->col) + 1) . ($twoCellAnchor->from->row + 1));
 

src/PhpSpreadsheet/Worksheet/BaseDrawing.php

@@ -192,7 +192,7 @@ public function setWorksheet(?Worksheet $worksheet = null, bool $overrideOld = f
     {
         if ($this->worksheet === null) {
             // Add drawing to \PhpOffice\PhpSpreadsheet\Worksheet\Worksheet
-            if ($worksheet !== null) {
+            if ($worksheet !== null && !($this instanceof Drawing && $this->getPath() === '')) {
                 $this->worksheet = $worksheet;
                 $this->worksheet->getCell($this->coordinates);
                 $this->worksheet->getDrawingCollection()->append($this);

src/PhpSpreadsheet/Worksheet/Drawing.php

@@ -94,40 +94,72 @@ public function getPath(): string
      */
     public function setPath(string $path, bool $verifyFile = true, ?ZipArchive $zip = null): static
     {
-        if ($verifyFile && preg_match('~^data:image/[a-z]+;base64,~', $path) !== 1) {
-            // Check if a URL has been passed. https://stackoverflow.com/a/2058596/1252979
-            if (filter_var($path, FILTER_VALIDATE_URL)) {
-                $this->path = $path;
-                // Implicit that it is a URL, rather store info than running check above on value in other places.
-                $this->isUrl = true;
-                $imageContents = file_get_contents($path);
+        $this->isUrl = false;
+        if (preg_match('~^data:image/[a-z]+;base64,~', $path) === 1) {
+            $this->path = $path;
+
+            return $this;
+        }
+
+        $this->path = '';
+        // Check if a URL has been passed. https://stackoverflow.com/a/2058596/1252979
+        if (filter_var($path, FILTER_VALIDATE_URL)) {
+            if (!preg_match('/^(http|https|file|ftp|s3):/', $path)) {
+                throw new PhpSpreadsheetException('Invalid protocol for linked drawing');
+            }
+            // Implicit that it is a URL, rather store info than running check above on value in other places.
+            $this->isUrl = true;
+            $imageContents = @file_get_contents($path);
+            if ($imageContents !== false) {
                 $filePath = tempnam(sys_get_temp_dir(), 'Drawing');
                 if ($filePath) {
-                    file_put_contents($filePath, $imageContents);
-                    if (file_exists($filePath)) {
-                        $this->setSizesAndType($filePath);
+                    $put = @file_put_contents($filePath, $imageContents);
+                    if ($put !== false) {
+                        if ($this->isImage($filePath)) {
+                            $this->path = $path;
+                            $this->setSizesAndType($filePath);
+                        }
                         unlink($filePath);
                     }
                 }
-            } elseif (file_exists($path)) {
-                $this->path = $path;
-                $this->setSizesAndType($path);
-            } elseif ($zip instanceof ZipArchive) {
-                $zipPath = explode('#', $path)[1];
-                if ($zip->locateName($zipPath) !== false) {
+            }
+        } elseif ($zip instanceof ZipArchive) {
+            $zipPath = explode('#', $path)[1];
+            $locate = @$zip->locateName($zipPath);
+            if ($locate !== false) {
+                if ($this->isImage($path)) {
                     $this->path = $path;
                     $this->setSizesAndType($path);
                 }
-            } else {
-                throw new PhpSpreadsheetException("File $path not found!");
             }
         } else {
-            $this->path = $path;
+            $exists = @file_exists($path);
+            if ($exists !== false && $this->isImage($path)) {
+                $this->path = $path;
+                $this->setSizesAndType($path);
+            }
+        }
+        if ($this->path === '' && $verifyFile) {
+            throw new PhpSpreadsheetException("File $path not found!");
         }
 
         return $this;
     }
 
+    private function isImage(string $path): bool
+    {
+        $mime = (string) @mime_content_type($path);
+        $retVal = false;
+        if (str_starts_with($mime, 'image/')) {
+            $retVal = true;
+        } elseif ($mime === 'application/octet-stream') {
+            $extension = pathinfo($path, PATHINFO_EXTENSION);
+            $retVal = in_array($extension, ['bin', 'emf'], true);
+        }
+
+        return $retVal;
+    }
+
     /**
      * Get isURL.
      */

src/PhpSpreadsheet/Writer/Html.php

@@ -578,6 +578,9 @@ private function extendRowsAndColumns(Worksheet $worksheet, int &$colMax, int &$
             }
         }
         foreach ($worksheet->getDrawingCollection() as $drawing) {
+            if ($drawing instanceof Drawing && $drawing->getPath() === '') {
+                continue;
+            }
             $imageTL = Coordinate::indexesFromString($drawing->getCoordinates());
             $this->sheetDrawings[$drawing->getCoordinates()] = $drawing;
             if ($imageTL[1] > $rowMax) {
@@ -620,7 +623,7 @@ private function writeImageInCell(string $coordinates): string
         if ($drawing !== null) {
             $filedesc = $drawing->getDescription();
             $filedesc = $filedesc ? htmlspecialchars($filedesc, ENT_QUOTES) : 'Embedded image';
-            if ($drawing instanceof Drawing) {
+            if ($drawing instanceof Drawing && $drawing->getPath() !== '') {
                 $filename = $drawing->getPath();
 
                 // Strip off eventual '.'
@@ -639,12 +642,15 @@ private function writeImageInCell(string $coordinates): string
                 $imageData = self::winFileToUrl($filename, $this instanceof Pdf\Mpdf);
 
                 if ($this->embedImages || str_starts_with($imageData, 'zip://')) {
+                    $imageData = 'data:,';
                     $picture = @file_get_contents($filename);
                     if ($picture !== false) {
-                        $imageDetails = getimagesize($filename) ?: ['mime' => ''];
-                        // base64 encode the binary data
-                        $base64 = base64_encode($picture);
-                        $imageData = 'data:' . $imageDetails['mime'] . ';base64,' . $base64;
+                        $mimeContentType = (string) @mime_content_type($filename);
+                        if (str_starts_with($mimeContentType, 'image/')) {
+                            // base64 encode the binary data
+                            $base64 = base64_encode($picture);
+                            $imageData = 'data:' . $mimeContentType . ';base64,' . $base64;
+                        }
                     }
                 }
 

src/PhpSpreadsheet/Writer/Xls.php

@@ -467,7 +467,7 @@ private function processDrawing(BstoreContainer &$bstoreContainer, Drawing $draw
 
     private function processBaseDrawing(BstoreContainer &$bstoreContainer, BaseDrawing $drawing): void
     {
-        if ($drawing instanceof Drawing) {
+        if ($drawing instanceof Drawing && $drawing->getPath() !== '') {
             $this->processDrawing($bstoreContainer, $drawing);
         } elseif ($drawing instanceof MemoryDrawing) {
             $this->processMemoryDrawing($bstoreContainer, $drawing, $drawing->getRenderingFunction());

src/PhpSpreadsheet/Writer/Xlsx.php

@@ -445,7 +445,9 @@ public function save($filename, int $flags = 0): void
 
                 // Media
                 foreach ($this->spreadSheet->getSheet($i)->getHeaderFooter()->getImages() as $image) {
-                    $zipContent['xl/media/' . $image->getIndexedFilename()] = file_get_contents($image->getPath());
+                    if ($image->getPath() !== '') {
+                        $zipContent['xl/media/' . $image->getIndexedFilename()] = file_get_contents($image->getPath());
+                    }
                 }
             }
 
@@ -461,6 +463,9 @@ public function save($filename, int $flags = 0): void
             if ($this->getDrawingHashTable()->getByIndex($i) instanceof WorksheetDrawing) {
                 $imageContents = null;
                 $imagePath = $this->getDrawingHashTable()->getByIndex($i)->getPath();
+                if ($imagePath === '') {
+                    continue;
+                }
                 if (str_contains($imagePath, 'zip://')) {
                     $imagePath = substr($imagePath, 6);
                     $imagePathSplitted = explode('#', $imagePath);
@@ -654,6 +659,9 @@ private function processDrawing(WorksheetDrawing $drawing): string|null|false
     {
         $data = null;
         $filename = $drawing->getPath();
+        if ($filename === '') {
+            return null;
+        }
         $imageData = getimagesize($filename);
 
         if (!empty($imageData)) {

src/PhpSpreadsheet/Writer/Xlsx/ContentTypes.php

@@ -6,6 +6,7 @@
 use PhpOffice\PhpSpreadsheet\Shared\File;
 use PhpOffice\PhpSpreadsheet\Shared\XMLWriter;
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheet\Worksheet\Drawing as WorksheetDrawing;
 use PhpOffice\PhpSpreadsheet\Worksheet\MemoryDrawing;
 use PhpOffice\PhpSpreadsheet\Writer\Exception as WriterException;
 
@@ -132,18 +133,23 @@ public function writeContentTypes(Spreadsheet $spreadsheet, bool $includeCharts
             $extension = '';
             $mimeType = '';
 
-            if ($this->getParentWriter()->getDrawingHashTable()->getByIndex($i) instanceof \PhpOffice\PhpSpreadsheet\Worksheet\Drawing) {
-                $extension = strtolower($this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getExtension());
-                $mimeType = $this->getImageMimeType($this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getPath());
-            } elseif ($this->getParentWriter()->getDrawingHashTable()->getByIndex($i) instanceof MemoryDrawing) {
-                $extension = strtolower($this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getMimeType());
+            $drawing = $this->getParentWriter()->getDrawingHashTable()->getByIndex($i);
+            if ($drawing instanceof WorksheetDrawing && $drawing->getPath() !== '') {
+                $extension = strtolower($drawing->getExtension());
+                if ($drawing->getIsUrl()) {
+                    $mimeType = image_type_to_mime_type($drawing->getType());
+                } else {
+                    $mimeType = $this->getImageMimeType($drawing->getPath());
+                }
+            } elseif ($drawing instanceof MemoryDrawing) {
+                $extension = strtolower($drawing->getMimeType());
                 $extension = explode('/', $extension);
                 $extension = $extension[1];
 
-                $mimeType = $this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getMimeType();
+                $mimeType = $drawing->getMimeType();
             }
 
-            if (!isset($aMediaContentTypes[$extension])) {
+            if ($mimeType !== '' && !isset($aMediaContentTypes[$extension])) {
                 $aMediaContentTypes[$extension] = $mimeType;
 
                 $this->writeDefaultContentType($objWriter, $extension, $mimeType);
@@ -162,7 +168,7 @@ public function writeContentTypes(Spreadsheet $spreadsheet, bool $includeCharts
         for ($i = 0; $i < $sheetCount; ++$i) {
             if (count($spreadsheet->getSheet($i)->getHeaderFooter()->getImages()) > 0) {
                 foreach ($spreadsheet->getSheet($i)->getHeaderFooter()->getImages() as $image) {
-                    if (!isset($aMediaContentTypes[strtolower($image->getExtension())])) {
+                    if ($image->getPath() !== '' && !isset($aMediaContentTypes[strtolower($image->getExtension())])) {
                         $aMediaContentTypes[strtolower($image->getExtension())] = $this->getImageMimeType($image->getPath());
 
                         $this->writeDefaultContentType($objWriter, strtolower($image->getExtension()), $aMediaContentTypes[strtolower($image->getExtension())]);

tests/PhpSpreadsheetTests/Reader/Xlsx/URLImageTest.php

@@ -4,14 +4,16 @@
 
 namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
 
+use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
 use PhpOffice\PhpSpreadsheet\IOFactory;
 use PhpOffice\PhpSpreadsheet\Worksheet\Drawing;
 use PhpOffice\PhpSpreadsheetTests\Reader\Utility\File;
 use PHPUnit\Framework\TestCase;
 
 class URLImageTest extends TestCase
 {
-    public function testURLImageSource(): void
+    // https://github.com/readthedocs/readthedocs.org/issues/11615
+    public function xtestURLImageSource(): void
     {
         if (getenv('SKIP_URL_IMAGE_TEST') === '1') {
             self::markTestSkipped('Skipped due to setting of environment variable');
@@ -41,4 +43,28 @@ public function testURLImageSource(): void
             self::assertSame('png', $extension);
         }
     }
+
+    public function xtestURLImageSourceNotFound(): void
+    {
+        if (getenv('SKIP_URL_IMAGE_TEST') === '1') {
+            self::markTestSkipped('Skipped due to setting of environment variable');
+        }
+        $filename = realpath(__DIR__ . '/../../../data/Reader/XLSX/urlImage.notfound.xlsx');
+        self::assertNotFalse($filename);
+        $reader = IOFactory::createReader('Xlsx');
+        $spreadsheet = $reader->load($filename);
+        $worksheet = $spreadsheet->getActiveSheet();
+        $collection = $worksheet->getDrawingCollection();
+        self::assertCount(0, $collection);
+    }
+
+    public function testURLImageSourceBadProtocol(): void
+    {
+        $filename = realpath(__DIR__ . '/../../../data/Reader/XLSX/urlImage.bad.dontuse');
+        self::assertNotFalse($filename);
+        $this->expectException(SpreadsheetException::class);
+        $this->expectExceptionMessage('Invalid protocol for linked drawing');
+        $reader = IOFactory::createReader('Xlsx');
+        $reader->load($filename);
+    }
 }

tests/PhpSpreadsheetTests/Writer/Html/ExtendForChartsAndImagesTest.php

@@ -4,6 +4,7 @@
 
 namespace PhpOffice\PhpSpreadsheetTests\Writer\Html;
 
+use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException;
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PhpOffice\PhpSpreadsheet\Worksheet\Drawing;
 use PhpOffice\PhpSpreadsheet\Writer\Html;
@@ -59,7 +60,9 @@ public function testSheetWithImageBelowData(): void
 
         // Add a drawing to the worksheet
         $drawing = new Drawing();
-        $drawing->setPath('foo.png', false);
+        $path = 'tests/data/Writer/XLSX/blue_square.png';
+        $drawing->setPath($path);
+        self::assertSame($path, $drawing->getPath());
         $drawing->setCoordinates('A5');
         $drawing->setWorksheet($sheet);
 
@@ -74,13 +77,51 @@ public function testSheetWithImageRightOfData(): void
 
         // Add a drawing to the worksheet
         $drawing = new Drawing();
-        $drawing->setPath('foo.png', false);
+        $path = 'tests/data/Writer/XLSX/blue_square.png';
+        $drawing->setPath($path);
+        self::assertSame($path, $drawing->getPath());
         $drawing->setCoordinates('E1');
         $drawing->setWorksheet($sheet);
 
         $this->assertMaxColumnAndMaxRow($spreadsheet, 5, 3);
     }
 
+    public function testSheetWithBadImageRightOfData(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->setCellValue('B3', 'foo');
+
+        // Add a drawing to the worksheet
+        $drawing = new Drawing();
+        $path = 'tests/data/Writer/XLSX/xblue_square.png';
+        $drawing->setPath($path, false);
+        self::assertSame('', $drawing->getPath());
+        $drawing->setCoordinates('E1');
+        $drawing->setWorksheet($sheet);
+
+        $this->assertMaxColumnAndMaxRow($spreadsheet, 2, 3);
+    }
+
+    public function testSheetWithBadImageRightOfDataThrow(): void
+    {
+        $this->expectException(PhpSpreadsheetException::class);
+        $this->expectExceptionMessage('not found!');
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->setCellValue('B3', 'foo');
+
+        // Add a drawing to the worksheet
+        $drawing = new Drawing();
+        $path = 'tests/data/Writer/XLSX/xblue_square.png';
+        $drawing->setPath($path);
+        self::assertSame('', $drawing->getPath());
+        $drawing->setCoordinates('E1');
+        $drawing->setWorksheet($sheet);
+
+        $this->assertMaxColumnAndMaxRow($spreadsheet, 2, 3);
+    }
+
     private function assertMaxColumnAndMaxRow(Spreadsheet $spreadsheet, int $expectedColumnCount, int $expectedRowCount): void
     {
         $writer = new Html($spreadsheet);