All Readers - Allow or Forbid Fetching of External Images Release129 (#4545)

oleibman · web-flow · commit 7c06eed662cc · 2025-07-22T21:40:30.000-07:00
* All Readers - Allow or Forbid Fetching of External Images

Add to all readers the option to allow or forbid fetching external images. This is unconditionally allowed now. The default will be set to "allow", so no code changes are necessary. However, we are giving consideration to changing the default.

* Update Changelog
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com)
 and this project adheres to [Semantic Versioning](https://semver.org).
 
+# 2025-07-23 - 1.29.12
+
+### Added
+
+- Add to all readers the option to allow or forbid fetching external images. This is unconditionally allowed now. The default will be set to "allow", so no code changes are necessary. However, we are giving consideration to changing the default.[PR #4545](https://github.com/PHPOffice/PhpSpreadsheet/pull/4545)
+
 # 2025-06-22 - 1.29.11
 
 ### Changed
diff --git a/src/PhpSpreadsheet/Reader/BaseReader.php b/src/PhpSpreadsheet/Reader/BaseReader.php
@@ -44,6 +44,15 @@ abstract class BaseReader implements IReader
      */
     protected $loadSheetsOnly;
 
+    /**
+     * Allow external images. Use with caution.
+     * Improper specification of these within a spreadsheet
+     * can subject the caller to security exploits.
+     *
+     * @var bool
+     */
+    protected $allowExternalImages = true;
+
     /**
      * IReadFilter instance.
      *
@@ -160,6 +169,12 @@ protected function processFlags(int $flags): void
         if (((bool) ($flags & self::SKIP_EMPTY_CELLS) || (bool) ($flags & self::IGNORE_EMPTY_CELLS)) === true) {
             $this->setReadEmptyCells(false);
         }
+        if (((bool) ($flags & self::ALLOW_EXTERNAL_IMAGES)) === true) {
+            $this->setAllowExternalImages(true);
+        }
+        if (((bool) ($flags & self::DONT_ALLOW_EXTERNAL_IMAGES)) === true) {
+            $this->setAllowExternalImages(false);
+        }
     }
 
     protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
@@ -203,4 +218,21 @@ protected function openFile(string $filename): void
 
         $this->fileHandle = $fileHandle;
     }
+
+    /**
+     * Allow external images. Use with caution.
+     * Improper specification of these within a spreadsheet
+     * can subject the caller to security exploits.
+     */
+    public function setAllowExternalImages(bool $allowExternalImages)
+    {
+        $this->allowExternalImages = $allowExternalImages;
+
+        return $this;
+    }
+
+    public function getAllowExternalImages()
+    {
+        return $this->allowExternalImages;
+    }
 }
diff --git a/src/PhpSpreadsheet/Reader/Html.php b/src/PhpSpreadsheet/Reader/Html.php
@@ -1084,7 +1084,7 @@ private function insertImage(Worksheet $sheet, $column, $row, array $attributes)
         $name = $attributes['alt'] ?? null;
 
         $drawing = new Drawing();
-        $drawing->setPath($src, false);
+        $drawing->setPath($src, false, null, $this->allowExternalImages);
         if ($drawing->getPath() === '') {
             return;
         }
diff --git a/src/PhpSpreadsheet/Reader/IReader.php b/src/PhpSpreadsheet/Reader/IReader.php
@@ -11,6 +11,14 @@ interface IReader
     public const SKIP_EMPTY_CELLS = 4;
     public const IGNORE_EMPTY_CELLS = 4;
 
+    /**
+     * Allow external images. Use with caution.
+     * Improper specification of these within a spreadsheet
+     * can subject the caller to security exploits.
+     */
+    public const ALLOW_EXTERNAL_IMAGES = 16;
+    public const DONT_ALLOW_EXTERNAL_IMAGES = 32;
+
     /**
      * IReader constructor.
      */
@@ -127,6 +135,22 @@ public function getReadFilter();
      */
     public function setReadFilter(IReadFilter $readFilter);
 
+    /**
+     * Allow external images. Use with caution.
+     * Improper specification of these within a spreadsheet
+     * can subject the caller to security exploits.
+     *
+     * @param bool $allowExternalImages
+     *
+     * @return IReader
+     */
+    public function setAllowExternalImages(bool $allowExternalImages);
+
+    /**
+     * @return bool
+     */
+    public function getAllowExternalImages();
+
     /**
      * Loads PhpSpreadsheet from file.
      *
@@ -136,6 +160,8 @@ public function setReadFilter(IReadFilter $readFilter);
      *            self::READ_DATA_ONLY      Read only data, not style or structure information, from the file
      *            self::SKIP_EMPTY_CELLS    Don't read empty cells (cells that contain a null value,
      *                                      empty string, or a string containing only whitespace characters)
+     *            self::ALLOW_EXTERNAL_IMAGES    Attempt to fetch images stored outside the spreadsheet.
+     *            self::DONT_ALLOW_EXTERNAL_IMAGES    Don't attempt to fetch images stored outside the spreadsheet.
      *
      * @return \PhpOffice\PhpSpreadsheet\Spreadsheet
      */
diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php
@@ -1436,7 +1436,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                                         );
                                                         if (isset($images[$linkImageKey])) {
                                                             $url = str_replace('xl/drawings/', '', $images[$linkImageKey]);
-                                                            $objDrawing->setPath($url, false);
+                                                            $objDrawing->setPath($url, false, null, $this->allowExternalImages);
                                                         }
                                                         if ($objDrawing->getPath() === '') {
                                                             continue;
@@ -1525,7 +1525,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                                         );
                                                         if (isset($images[$linkImageKey])) {
                                                             $url = str_replace('xl/drawings/', '', $images[$linkImageKey]);
-                                                            $objDrawing->setPath($url, false);
+                                                            $objDrawing->setPath($url, false, null, $this->allowExternalImages);
                                                         }
                                                         if ($objDrawing->getPath() === '') {
                                                             continue;
diff --git a/src/PhpSpreadsheet/Worksheet/Drawing.php b/src/PhpSpreadsheet/Worksheet/Drawing.php
@@ -101,10 +101,11 @@ public function getPath()
      * @param string $path File path
      * @param bool $verifyFile Verify file
      * @param ZipArchive $zip Zip archive instance
+     * @param bool $allowExternal
      *
      * @return $this
      */
-    public function setPath($path, $verifyFile = true, $zip = null)
+    public function setPath($path, $verifyFile = true, $zip = null, $allowExternal = true)
     {
         $this->isUrl = false;
         if (preg_match('~^data:image/[a-z]+;base64,~', $path) === 1) {
@@ -119,6 +120,9 @@ public function setPath($path, $verifyFile = true, $zip = null)
             if (!preg_match('/^(http|https|file|ftp|s3):/', $path)) {
                 throw new PhpSpreadsheetException('Invalid protocol for linked drawing');
             }
+            if (!$allowExternal) {
+                return $this;
+            }
             // Implicit that it is a URL, rather store info than running check above on value in other places.
             $this->isUrl = true;
             $ctx = null;
diff --git a/tests/PhpSpreadsheetTests/Reader/Html/HtmlHelper.php b/tests/PhpSpreadsheetTests/Reader/Html/HtmlHelper.php
@@ -16,9 +16,12 @@ public static function createHtml(string $html): string
         return $filename;
     }
 
-    public static function loadHtmlIntoSpreadsheet(string $filename, bool $unlink = false): Spreadsheet
+    public static function loadHtmlIntoSpreadsheet(string $filename, bool $unlink = false, ?bool $allowExternalImages = null): Spreadsheet
     {
         $html = new Html();
+        if ($allowExternalImages !== null) {
+            $html->setAllowExternalImages($allowExternalImages);
+        }
         $spreadsheet = $html->load($filename);
         if ($unlink) {
             unlink($filename);
diff --git a/tests/PhpSpreadsheetTests/Reader/Html/HtmlImage2Test.php b/tests/PhpSpreadsheetTests/Reader/Html/HtmlImage2Test.php
@@ -11,7 +11,7 @@
 
 class HtmlImage2Test extends TestCase
 {
-    public function testCanInsertImageGoodProtocol(): void
+    public function testCanInsertImageGoodProtocolAllowed(): void
     {
         if (getenv('SKIP_URL_IMAGE_TEST') === '1') {
             self::markTestSkipped('Skipped due to setting of environment variable');
@@ -23,16 +23,32 @@ public function testCanInsertImageGoodProtocol(): void
                     </tr>
                 </table>';
         $filename = HtmlHelper::createHtml($html);
-        $spreadsheet = HtmlHelper::loadHtmlIntoSpreadsheet($filename, true);
+        $spreadsheet = HtmlHelper::loadHtmlIntoSpreadsheet($filename, true, true);
         $firstSheet = $spreadsheet->getSheet(0);
 
         /** @var Drawing $drawing */
         $drawing = $firstSheet->getDrawingCollection()[0];
         self::assertEquals($imagePath, $drawing->getPath());
         self::assertEquals('A1', $drawing->getCoordinates());
+        $spreadsheet->disconnectWorksheets();
     }
 
-    public function testCantInsertImageNotFound(): void
+    public function testCanInsertImageGoodProtocolNotAllowed(): void
+    {
+        $imagePath = 'https://phpspreadsheet.readthedocs.io/en/latest/topics/images/01-03-filter-icon-1.png';
+        $html = '<table>
+                    <tr>
+                        <td><img src="' . $imagePath . '" alt="test image voilà"></td>
+                    </tr>
+                </table>';
+        $filename = HtmlHelper::createHtml($html);
+        $spreadsheet = HtmlHelper::loadHtmlIntoSpreadsheet($filename, true, false);
+        $firstSheet = $spreadsheet->getSheet(0);
+        self::assertCount(0, $firstSheet->getDrawingCollection());
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testCantInsertImageNotFoundAllowed(): void
     {
         if (getenv('SKIP_URL_IMAGE_TEST') === '1') {
             self::markTestSkipped('Skipped due to setting of environment variable');
@@ -44,10 +60,27 @@ public function testCantInsertImageNotFound(): void
                     </tr>
                 </table>';
         $filename = HtmlHelper::createHtml($html);
-        $spreadsheet = HtmlHelper::loadHtmlIntoSpreadsheet($filename, true);
+        $spreadsheet = HtmlHelper::loadHtmlIntoSpreadsheet($filename, true, true);
+        $firstSheet = $spreadsheet->getSheet(0);
+        $drawingCollection = $firstSheet->getDrawingCollection();
+        self::assertCount(0, $drawingCollection);
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testCantInsertImageNotFoundNotAllowed(): void
+    {
+        $imagePath = 'https://phpspreadsheet.readthedocs.io/en/latest/topics/images/xxx01-03-filter-icon-1.png';
+        $html = '<table>
+                    <tr>
+                        <td><img src="' . $imagePath . '" alt="test image voilà"></td>
+                    </tr>
+                </table>';
+        $filename = HtmlHelper::createHtml($html);
+        $spreadsheet = HtmlHelper::loadHtmlIntoSpreadsheet($filename, true, false);
         $firstSheet = $spreadsheet->getSheet(0);
         $drawingCollection = $firstSheet->getDrawingCollection();
         self::assertCount(0, $drawingCollection);
+        $spreadsheet->disconnectWorksheets();
     }
 
     /**
diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/URLImageTest.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/URLImageTest.php
@@ -10,14 +10,15 @@
 
 class URLImageTest extends TestCase
 {
-    public function testURLImageSource(): void
+    public function testURLImageSourceAllowed(): void
     {
         if (getenv('SKIP_URL_IMAGE_TEST') === '1') {
             self::markTestSkipped('Skipped due to setting of environment variable');
         }
         $filename = realpath(__DIR__ . '/../../../data/Reader/XLSX/urlImage.xlsx');
         self::assertNotFalse($filename);
         $reader = IOFactory::createReader('Xlsx');
+        $reader->setAllowExternalImages(true);
         $spreadsheet = $reader->load($filename);
         $worksheet = $spreadsheet->getActiveSheet();
         $collection = $worksheet->getDrawingCollection();
@@ -32,17 +33,47 @@ public function testURLImageSource(): void
             self::assertSame(84, $drawing->getWidth());
             self::assertSame(44, $drawing->getHeight());
         }
-         $spreadsheet->disconnectWorksheets();
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testURLImageSourceNotAllowed(): void
+    {
+        $filename = realpath(__DIR__ . '/../../../data/Reader/XLSX/urlImage.xlsx');
+        self::assertNotFalse($filename);
+        $reader = IOFactory::createReader('Xlsx');
+        $reader->setAllowExternalImages(false);
+        $spreadsheet = $reader->load($filename);
+        $worksheet = $spreadsheet->getActiveSheet();
+        $collection = $worksheet->getDrawingCollection();
+        self::assertCount(0, $collection);
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testURLImageSourceNotFoundAllowed(): 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');
+        $reader->setAllowExternalImages(true);
+        $spreadsheet = $reader->load($filename);
+        $worksheet = $spreadsheet->getActiveSheet();
+        $collection = $worksheet->getDrawingCollection();
+        self::assertCount(0, $collection);
+        $spreadsheet->disconnectWorksheets();
     }
 
-    public function testURLImageSourceNotFound(): void
+    public function testURLImageSourceNotFoundNotAllowed(): 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');
+        $reader->setAllowExternalImages(false);
         $spreadsheet = $reader->load($filename);
         $worksheet = $spreadsheet->getActiveSheet();
         $collection = $worksheet->getDrawingCollection();

Original file line number	Diff line number	Diff line change
`@@ -1084,7 +1084,7 @@ private function insertImage(Worksheet $sheet, $column, $row, array $attributes)`
`1084`	`1084`	`$name = $attributes['alt'] ?? null;`
`1085`	`1085`
`1086`	`1086`	`$drawing = new Drawing();`
`1087`		`- $drawing->setPath($src, false);`
	`1087`	`+ $drawing->setPath($src, false, null, $this->allowExternalImages);`
`1088`	`1088`	`if ($drawing->getPath() === '') {`
`1089`	`1089`	`return;`
`1090`	`1090`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,9 +16,12 @@ public static function createHtml(string $html): string`
`16`	`16`	`return $filename;`
`17`	`17`	`}`
`18`	`18`
`19`		`- public static function loadHtmlIntoSpreadsheet(string $filename, bool $unlink = false): Spreadsheet`
	`19`	`+ public static function loadHtmlIntoSpreadsheet(string $filename, bool $unlink = false, ?bool $allowExternalImages = null): Spreadsheet`
`20`	`20`	`{`
`21`	`21`	`$html = new Html();`
	`22`	`+ if ($allowExternalImages !== null) {`
	`23`	`+ $html->setAllowExternalImages($allowExternalImages);`
	`24`	`+ }`
`22`	`25`	`$spreadsheet = $html->load($filename);`
`23`	`26`	`if ($unlink) {`
`24`	`27`	`unlink($filename);`