Security Patch Special Characters in Protocol

PHPOffice · Jan 24, 2025 · f68f688 · f68f688
1 parent 0a3d70c
commit f68f688
Show file tree

Hide file tree

Showing 17 changed files with 488 additions and 149 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,13 @@ 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).
 
+# TBD - 2.1.8
+
+### Fixed
+
+- Backported security patch for control characters in protocol.
+- Use Composer\Pcre in Xls/Parser. Partial backport of [PR #4203](https://github.com/PHPOffice/PhpSpreadsheet/pull/4203)
+
 # 2025-01-11 - 2.1.7
 
 ### Deprecated

diff --git a/composer.json b/composer.json
@@ -79,6 +79,7 @@
         "ext-xmlwriter": "*",
         "ext-zip": "*",
         "ext-zlib": "*",
+        "composer/pcre": "^3.2",
         "maennchen/zipstream-php": "^2.1 || ^3.0",
         "markbaker/complex": "^3.0",
         "markbaker/matrix": "^3.0",

diff --git a/composer.lock b/composer.lock
diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -2,6 +2,7 @@ includes:
     - phpstan-baseline.neon
     - vendor/phpstan/phpstan-phpunit/extension.neon
     - vendor/phpstan/phpstan-phpunit/rules.neon
+    - vendor/composer/pcre/extension.neon
 
 parameters:
     level: 8
@@ -22,9 +23,9 @@ parameters:
         - src/PhpSpreadsheet/Writer/ZipStream3.php
     parallel:
         processTimeout: 300.0
-    checkMissingIterableValueType: false
-    checkGenericClassInNonGenericObjectType: false
     ignoreErrors:
+        - identifier: missingType.iterableValue
+        - identifier: missingType.generics
         # Accept a bit anything for assert methods
         - '~^Parameter \#2 .* of static method PHPUnit\\Framework\\Assert\:\:assert\w+\(\) expects .*, .* given\.$~'
         - '~^Variable \$helper might not be defined\.$~'
diff --git a/src/PhpSpreadsheet/Calculation/DateTimeExcel/Helpers.php b/src/PhpSpreadsheet/Calculation/DateTimeExcel/Helpers.php
@@ -19,6 +19,8 @@ class Helpers
      */
     public static function isLeapYear(int|string $year): bool
     {
+        $year = (int) $year;
+
         return (($year % 4) === 0) && (($year % 100) !== 0) || (($year % 400) === 0);
     }
 

diff --git a/src/PhpSpreadsheet/Calculation/DateTimeExcel/TimeValue.php b/src/PhpSpreadsheet/Calculation/DateTimeExcel/TimeValue.php
@@ -52,7 +52,7 @@ public static function fromString(null|array|string|int|bool|float $timeValue):
 
         $arraySplit = preg_split('/[\/:\-\s]/', $timeValue) ?: [];
         if ((count($arraySplit) == 2 || count($arraySplit) == 3) && $arraySplit[0] > 24) {
-            $arraySplit[0] = ($arraySplit[0] % 24);
+            $arraySplit[0] = ($arraySplit[0] % 24); // @phpstan-ignore-line
             $timeValue = implode(':', $arraySplit);
         }
 

diff --git a/src/PhpSpreadsheet/Helper/Html.php b/src/PhpSpreadsheet/Helper/Html.php
@@ -693,7 +693,7 @@ private function rgbToColour(string $rgbValue): string
     {
         preg_match_all('/\d+/', $rgbValue, $values);
         foreach ($values[0] as &$value) {
-            $value = str_pad(dechex($value), 2, '0', STR_PAD_LEFT);
+            $value = str_pad(dechex($value), 2, '0', STR_PAD_LEFT); // @phpstan-ignore-line
         }
 
         return implode('', $values[0]);

diff --git a/src/PhpSpreadsheet/Reader/Html.php b/src/PhpSpreadsheet/Reader/Html.php
@@ -170,7 +170,7 @@ private function readBeginning(): string
     private function readEnding(): string
     {
         $meta = stream_get_meta_data($this->fileHandle);
-        $filename = $meta['uri'];
+        $filename = $meta['uri']; // @phpstan-ignore-line
 
         $size = (int) filesize($filename);
         if ($size === 0) {
@@ -1031,7 +1031,10 @@ private function insertImage(Worksheet $sheet, string $column, int $row, array $
         $name = $attributes['alt'] ?? null;
 
         $drawing = new Drawing();
-        $drawing->setPath($src);
+        $drawing->setPath($src, false);
+        if ($drawing->getPath() === '') {
+            return;
+        }
         $drawing->setWorksheet($sheet);
         $drawing->setCoordinates($column . $row);
         $drawing->setOffsetX(0);

diff --git a/src/PhpSpreadsheet/Reader/Slk.php b/src/PhpSpreadsheet/Reader/Slk.php
@@ -447,7 +447,7 @@ private function processPRecord(array $rowData, Spreadsheet &$spreadsheet): void
     private function processPColors(string $rowDatum, array &$formatArray): void
     {
         if (preg_match('/L([1-9]\\d*)/', $rowDatum, $matches)) {
-            $fontColor = $matches[1] % 8;
+            $fontColor = $matches[1] % 8; // @phpstan-ignore-line
             $formatArray['font']['color']['argb'] = self::COLOR_ARRAY[$fontColor];
         }
     }

diff --git a/src/PhpSpreadsheet/Worksheet/Drawing.php b/src/PhpSpreadsheet/Worksheet/Drawing.php
@@ -103,7 +103,7 @@ public function setPath(string $path, bool $verifyFile = true, ?ZipArchive $zip
 
         $this->path = '';
         // Check if a URL has been passed. https://stackoverflow.com/a/2058596/1252979
-        if (filter_var($path, FILTER_VALIDATE_URL)) {
+        if (filter_var($path, FILTER_VALIDATE_URL) || (preg_match('/^([\\w\\s\\x00-\\x1f]+):/u', $path) && !preg_match('/^([\\w]+):/u', $path))) {
             if (!preg_match('/^(http|https|file|ftp|s3):/', $path)) {
                 throw new PhpSpreadsheetException('Invalid protocol for linked drawing');
             }

diff --git a/src/PhpSpreadsheet/Worksheet/MemoryDrawing.php b/src/PhpSpreadsheet/Worksheet/MemoryDrawing.php
@@ -102,7 +102,7 @@ private function cloneResource(): void
             $transparent = imagecolortransparent($this->imageResource);
             if ($transparent >= 0) {
                 $rgb = imagecolorsforindex($this->imageResource, $transparent);
-                if (empty($rgb)) {
+                if (empty($rgb)) { // @phpstan-ignore-line
                     throw new Exception('Could not get image colors');
                 }
 

diff --git a/src/PhpSpreadsheet/Writer/Html.php b/src/PhpSpreadsheet/Writer/Html.php
@@ -2,6 +2,7 @@
 
 namespace PhpOffice\PhpSpreadsheet\Writer;
 
+use Composer\Pcre\Preg;
 use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
 use PhpOffice\PhpSpreadsheet\Cell\Cell;
 use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
@@ -621,13 +622,13 @@ private function writeImageInCell(string $coordinates): string
                 $filename = $drawing->getPath();
 
                 // Strip off eventual '.'
-                $filename = (string) preg_replace('/^[.]/', '', $filename);
+                $filename = Preg::replace('/^[.]/', '', $filename);
 
                 // Prepend images root
                 $filename = $this->getImagesRoot() . $filename;
 
                 // Strip off eventual '.' if followed by non-/
-                $filename = (string) preg_replace('@^[.]([^/])@', '$1', $filename);
+                $filename = Preg::replace('@^[.]([^/])@', '$1', $filename);
 
                 // Convert UTF8 data to PCDATA
                 $filename = htmlspecialchars($filename, Settings::htmlEntityFlags());
@@ -1350,7 +1351,7 @@ private function generateRowCellData(Worksheet $worksheet, null|Cell|string $cel
 
             // Converts the cell content so that spaces occuring at beginning of each new line are replaced by &nbsp;
             // Example: "  Hello\n to the world" is converted to "&nbsp;&nbsp;Hello\n&nbsp;to the world"
-            $cellData = (string) preg_replace('/(?m)(?:^|\\G) /', '&nbsp;', $cellData);
+            $cellData = Preg::replace('/(?m)(?:^|\\G) /', '&nbsp;', $cellData);
 
             // convert newline "\n" to '<br>'
             $cellData = nl2br($cellData);
@@ -1495,10 +1496,11 @@ private function generateRow(Worksheet $worksheet, array $values, int $row, stri
             if ($worksheet->hyperlinkExists($coordinate) && !$worksheet->getHyperlink($coordinate)->isInternal()) {
                 $url = $worksheet->getHyperlink($coordinate)->getUrl();
                 $urlDecode1 = html_entity_decode($url, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
-                $urlTrim = preg_replace('/^\\s+/u', '', $urlDecode1) ?? $urlDecode1;
-                $parseScheme = preg_match('/^([\\w\\s]+):/u', strtolower($urlTrim), $matches);
-                if ($parseScheme === 1 && !in_array($matches[1], ['http', 'https', 'file', 'ftp', 's3'], true)) {
+                $urlTrim = Preg::replace('/^\\s+/u', '', $urlDecode1);
+                $parseScheme = Preg::isMatch('/^([\\w\\s\\x00-\\x1f]+):/u', strtolower($urlTrim), $matches);
+                if ($parseScheme && !in_array($matches[1], ['http', 'https', 'file', 'ftp', 'mailto', 's3'], true)) {
                     $cellData = htmlspecialchars($url, Settings::htmlEntityFlags());
+                    $cellData = self::replaceControlChars($cellData);
                 } else {
                     $cellData = '<a href="' . htmlspecialchars($url, Settings::htmlEntityFlags()) . '" title="' . htmlspecialchars($worksheet->getHyperlink($coordinate)->getTooltip(), Settings::htmlEntityFlags()) . '">' . $cellData . '</a>';
                 }
@@ -1540,6 +1542,20 @@ private function generateRow(Worksheet $worksheet, array $values, int $row, stri
         return $html;
     }
 
+    public static function replaceNonAscii(array $matches): string
+    {
+        return '&#' . mb_ord($matches[0], 'UTF-8') . ';';
+    }
+
+    private static function replaceControlChars(string $convert): string
+    {
+        return Preg::replaceCallback(
+            '/[\\x00-\\x1f]/',
+            [self::class, 'replaceNonAscii'],
+            $convert
+        );
+    }
+
     /**
      * Takes array where of CSS properties / values and converts to CSS string.
      */
@@ -1627,8 +1643,8 @@ public function formatColor(string $value, string $format): string
         $matches = [];
 
         $color_regex = '/^\\[[a-zA-Z]+\\]/';
-        if (preg_match($color_regex, $format, $matches)) {
-            $color = str_replace(['[', ']'], '', $matches[0]);
+        if (Preg::isMatch($color_regex, $format, $matches)) {
+            $color = str_replace(['[', ']'], '', $matches[0]); // @phpstan-ignore-line
             $color = strtolower($color);
         }
-Original file line number
+Diff line change
@@ Expand Up / @@ -19,6 +19,8 @@ class Helpers @@
          */
         public static function isLeapYear(int|string $year): bool
         {
+            $year = (int) $year;
             return (($year % 4) === 0) && (($year % 100) !== 0) || (($year % 400) === 0);
         }
@@ Expand Down @@