All Readers - Allow or Forbid Fetching of External Images Release390 (#4548)

oleibman · web-flow · commit 668d3b5fdebf · 2025-07-22T21:44:46.000-07:00
* All Readers - Allow or Forbid Fetching of External Images Release390

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 - 3.9.3
+
+### 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 #4548](https://github.com/PHPOffice/PhpSpreadsheet/pull/4548)
+
 # 2025-06-22 - 3.9.2
 
 ### Changed
diff --git a/src/PhpSpreadsheet/Reader/BaseReader.php b/src/PhpSpreadsheet/Reader/BaseReader.php
@@ -47,6 +47,13 @@ abstract class BaseReader implements IReader
      */
     protected bool $ignoreRowsWithNoCells = false;
 
+    /**
+     * Allow external images. Use with caution.
+     * Improper specification of these within a spreadsheet
+     * can subject the caller to security exploits.
+     */
+    protected bool $allowExternalImages = true;
+
     /**
      * IReadFilter instance.
      */
@@ -147,6 +154,23 @@ public function setReadFilter(IReadFilter $readFilter): self
         return $this;
     }
 
+    /**
+     * 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): self
+    {
+        $this->allowExternalImages = $allowExternalImages;
+
+        return $this;
+    }
+
+    public function getAllowExternalImages(): bool
+    {
+        return $this->allowExternalImages;
+    }
+
     public function getSecurityScanner(): ?XmlScanner
     {
         return $this->securityScanner;
@@ -175,6 +199,12 @@ protected function processFlags(int $flags): void
         if (((bool) ($flags & self::IGNORE_ROWS_WITH_NO_CELLS)) === true) {
             $this->setIgnoreRowsWithNoCells(true);
         }
+        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
diff --git a/src/PhpSpreadsheet/Reader/Html.php b/src/PhpSpreadsheet/Reader/Html.php
@@ -1090,7 +1090,7 @@ private function insertImage(Worksheet $sheet, string $column, int $row, array $
         $name = $attributes['alt'] ?? null;
 
         $drawing = new Drawing();
-        $drawing->setPath($src, false);
+        $drawing->setPath($src, false, allowExternal: $this->allowExternalImages);
         if ($drawing->getPath() === '') {
             return;
         }
diff --git a/src/PhpSpreadsheet/Reader/IReader.php b/src/PhpSpreadsheet/Reader/IReader.php
@@ -38,6 +38,14 @@ interface IReader
      */
     public const IGNORE_ROWS_WITH_NO_CELLS = 8;
 
+    /**
+     * 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;
+
     public function __construct();
 
     /**
@@ -128,6 +136,15 @@ public function setLoadAllSheets(): self;
      */
     public function getReadFilter(): IReadFilter;
 
+    /**
+     * 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): self;
+
+    public function getAllowExternalImages(): bool;
+
     /**
      * Set read filter.
      *
@@ -144,6 +161,9 @@ public function setReadFilter(IReadFilter $readFilter): self;
      *            self::READ_DATA_ONLY      Read only data, not style or structure information, from the file
      *            self::IGNORE_EMPTY_CELLS  Don't read empty cells (cells that contain a null value,
      *                                      empty string, or a string containing only whitespace characters)
+     *            self::IGNORE_ROWS_WITH_NO_CELLS    Don't load any rows that contain no cells.
+     *            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.
      */
     public function load(string $filename, int $flags = 0): Spreadsheet;
 }
diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php
@@ -1481,7 +1481,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, allowExternal: $this->allowExternalImages);
                                                         }
                                                         if ($objDrawing->getPath() === '') {
                                                             continue;
@@ -1579,7 +1579,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, allowExternal: $this->allowExternalImages);
                                                         }
                                                         if ($objDrawing->getPath() === '') {
                                                             continue;
diff --git a/src/PhpSpreadsheet/Worksheet/Drawing.php b/src/PhpSpreadsheet/Worksheet/Drawing.php
@@ -92,7 +92,7 @@ public function getPath(): string
      *
      * @return $this
      */
-    public function setPath(string $path, bool $verifyFile = true, ?ZipArchive $zip = null): static
+    public function setPath(string $path, bool $verifyFile = true, ?ZipArchive $zip = null, bool $allowExternal = true): static
     {
         $this->isUrl = false;
         if (preg_match('~^data:image/[a-z]+;base64,~', $path) === 1) {
@@ -107,6 +107,9 @@ public function setPath(string $path, bool $verifyFile = true, ?ZipArchive $zip
             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
@@ -18,9 +18,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,34 @@ 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);
+
+        $drawings = $firstSheet->getDrawingCollection();
+        self::assertCount(0, $drawings);
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testCantInsertImageNotFoundAllowed(): void
     {
         if (getenv('SKIP_URL_IMAGE_TEST') === '1') {
             self::markTestSkipped('Skipped due to setting of environment variable');
@@ -44,10 +62,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, true);
         $firstSheet = $spreadsheet->getSheet(0);
         $drawingCollection = $firstSheet->getDrawingCollection();
         self::assertCount(0, $drawingCollection);
+        $spreadsheet->disconnectWorksheets();
     }
 
     #[DataProvider('providerBadProtocol')]
diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/URLImageTest.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/URLImageTest.php
@@ -12,14 +12,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();
@@ -37,14 +38,41 @@ public function testURLImageSource(): void
         $spreadsheet->disconnectWorksheets();
     }
 
-    public function testURLImageSourceNotFound(): void
+    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 testURLImageSourceNotFoundNotAllowed(): void
+    {
+        $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
`@@ -1090,7 +1090,7 @@ private function insertImage(Worksheet $sheet, string $column, int $row, array $`
`1090`	`1090`	`$name = $attributes['alt'] ?? null;`
`1091`	`1091`
`1092`	`1092`	`$drawing = new Drawing();`
`1093`		`- $drawing->setPath($src, false);`
	`1093`	`+ $drawing->setPath($src, false, allowExternal: $this->allowExternalImages);`
`1094`	`1094`	`if ($drawing->getPath() === '') {`
`1095`	`1095`	`return;`
`1096`	`1096`	`}`
Original file line number	Diff line number	Diff line change
`@@ -92,7 +92,7 @@ public function getPath(): string`
`92`	`92`	`*`
`93`	`93`	`* @return $this`
`94`	`94`	`*/`
`95`		`- public function setPath(string $path, bool $verifyFile = true, ?ZipArchive $zip = null): static`
	`95`	`+ public function setPath(string $path, bool $verifyFile = true, ?ZipArchive $zip = null, bool $allowExternal = true): static`
`96`	`96`	`{`
`97`	`97`	`$this->isUrl = false;`
`98`	`98`	`if (preg_match('~^data:image/[a-z]+;base64,~', $path) === 1) {`
`@@ -107,6 +107,9 @@ public function setPath(string $path, bool $verifyFile = true, ?ZipArchive $zip`
`107`	`107`	`if (!preg_match('/^(http\|https\|file\|ftp\|s3):/', $path)) {`
`108`	`108`	`throw new PhpSpreadsheetException('Invalid protocol for linked drawing');`
`109`	`109`	`}`
	`110`	`+ if (!$allowExternal) {`
	`111`	`+ return $this;`
	`112`	`+ }`
`110`	`113`	`// Implicit that it is a URL, rather store info than running check above on value in other places.`
`111`	`114`	`$this->isUrl = true;`
`112`	`115`	`$ctx = null;`
Original file line number	Diff line number	Diff line change
`@@ -18,9 +18,12 @@ public static function createHtml(string $html): string`
`18`	`18`	`return $filename;`
`19`	`19`	`}`
`20`	`20`
`21`		`- public static function loadHtmlIntoSpreadsheet(string $filename, bool $unlink = false): Spreadsheet`
	`21`	`+ public static function loadHtmlIntoSpreadsheet(string $filename, bool $unlink = false, ?bool $allowExternalImages = null): Spreadsheet`
`22`	`22`	`{`
`23`	`23`	`$html = new Html();`
	`24`	`+ if ($allowExternalImages !== null) {`
	`25`	`+ $html->setAllowExternalImages($allowExternalImages);`
	`26`	`+ }`
`24`	`27`	`$spreadsheet = $html->load($filename);`
`25`	`28`	`if ($unlink) {`
`26`	`29`	`unlink($filename);`