Backport Security Patch

PHPOffice · Nov 10, 2024 · 31d7f79 · 31d7f79
1 parent 15e028f
commit 31d7f79
Show file tree

Hide file tree

Showing 20 changed files with 293 additions and 110 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -13,6 +13,7 @@ jobs:
           - '8.1'
           - '8.2'
           - '8.3'
+          - '8.4'
 
         include:
           - php-version: 'nightly'
@@ -44,7 +45,7 @@ jobs:
 
       - name: Delete composer lock file
         id: composer-lock
-        if: ${{ matrix.php-version == '8.1' || matrix.php-version == '8.2' || matrix.php-version == '8.3' || matrix.php-version == 'nightly'}}
+        if: ${{ matrix.php-version == '8.1' || matrix.php-version == '8.2' || matrix.php-version == '8.3' || matrix.php-version == '8.4' || matrix.php-version == 'nightly'}}
         run: |
           rm composer.lock
           echo "flags=--ignore-platform-reqs" >> $GITHUB_OUTPUT
@@ -67,9 +68,27 @@ jobs:
       - name: "Run PHPUnit tests 2 (Experimental: ${{ matrix.experimental }})"
         env:
           FAILURE_ACTION: "${{ matrix.experimental == true }}"
-        if: ${{ matrix.php-version == '8.1' || matrix.php-version == '8.2' || matrix.php-version == '8.3' || matrix.php-version == 'nightly'}}
+        if: ${{ matrix.php-version == '8.1' || matrix.php-version == '8.2' || matrix.php-version == '8.3' || matrix.php-version == '8.4' || matrix.php-version == 'nightly'}}
         run: vendor/bin/phpunit --verbose || $FAILURE_ACTION
 
+  find-polyfill:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 2
+
+      - name: Setup PHP, with composer and extensions
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: 8.3
+          extensions: ctype, dom, gd, iconv, fileinfo, libxml, mbstring, simplexml, xml, xmlreader, xmlwriter, zip, zlib
+          coverage: none
+
+      - name: Find code that might require polyfill
+        run: php ./bin/findpolyfill.php
+
   php-cs-fixer:
     runs-on: ubuntu-latest
     steps:

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,11 +5,18 @@ 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 - 1.29.3
+## 1.29.3 - 2024-11-10
+
+### Fixed
+
+- Backported security patches.
+- Write ignoredErrors Tag Before Drawings. Backport of [PR #4212](https://github.com/PHPOffice/PhpSpreadsheet/pull/4212) intended for 3.4.0.
+- Changes to ROUNDDOWN/ROUNDUP/TRUNC. Backport of [PR #4214](https://github.com/PHPOffice/PhpSpreadsheet/pull/4214) intended for 3.4.0.
+- Replace str_starts_with in Drawing. [Issue #4215](https://github.com/PHPOffice/PhpSpreadsheet/issues/4215)
 
 ### Added
 
-- Method to Test Whether Csv Will Be Affected by Php9 (backport of PR #4189 intended for 3.4.0)
+- Method to Test Whether Csv Will Be Affected by Php9. Backport of [PR #4189](https://github.com/PHPOffice/PhpSpreadsheet/pull/4189)  intended for 3.4.0.
 
 ## 1.29.2 - 2024-09-29
 

diff --git a/bin/findpolyfill.php b/bin/findpolyfill.php
@@ -0,0 +1,54 @@
+#!/usr/bin/env php
+<?php
+
+function findPolyfill(string $directory): int
+{
+    // See issue #4215 - code which should have erred in unit test
+    // succeeded because a dev package required polyfills.
+    $retCode = 0;
+    $polyfill80 = 'FILTER_VALIDATE_BOOL\\b'
+        . '|fdiv[(]'
+        . '|preg_last_error_msg[(]'
+        . '|str_contains[(]'
+        . '|str_starts_with[(]'
+        . '|str_ends_with[(]'
+        . '|get_debug_type[(]'
+        . '|get_resource_id[(]';
+    $polyfill81 = 'MYSQLI_REFRESH_REPLICA\\b'
+        . '|fdiv[(]'
+        . '|array_is_list[(]'
+        . '|enum_exists[(]';
+    $polyfill = '/\\b(?:'
+        . $polyfill80
+        . '|'
+        . $polyfill81
+        . '})/';
+
+    $it = new RecursiveIteratorIterator(
+        new RecursiveDirectoryIterator($directory, FilesystemIterator::UNIX_PATHS)
+    );
+
+    foreach ($it as $file) {
+        if ($file->getExtension() === 'php') {
+            $fullname = $it->getPath() . '/' . $it->getBaseName();
+            $contents = file_get_contents($fullname);
+            if ($contents === false) {
+                echo "failed to read $fullname\n";
+                ++$retCode;
+            } elseif (preg_match_all($polyfill, $contents, $matches)) {
+                var_dump($fullname, $matches);
+                ++$retCode;
+            }
+        }
+    }
+
+    return $retCode;
+}
+
+// Don't care if tests use polyfill
+$errors = findPolyfill(__DIR__ . '/../src') + findPolyfill(__DIR__ . '/../samples');
+if ($errors !== 0) {
+    echo "Found $errors files that might require polyfills\n";
+    exit(1);
+}
+echo "No polyfills needed\n";
diff --git a/src/PhpSpreadsheet/Calculation/MathTrig/Round.php b/src/PhpSpreadsheet/Calculation/MathTrig/Round.php
@@ -5,6 +5,8 @@
 use PhpOffice\PhpSpreadsheet\Calculation\ArrayEnabled;
 use PhpOffice\PhpSpreadsheet\Calculation\Exception;
 use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
+// following added in Php8.4
+use RoundingMode;
 
 class Round
 {
@@ -67,31 +69,28 @@ public static function up($number, $digits)
             return 0.0;
         }
 
-        $digitsPlus1 = $digits + 1;
-        if ($number < 0.0) {
-            if ($digitsPlus1 < 0) {
-                return round($number - 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_DOWN);
-            }
-            $result = sprintf("%.{$digitsPlus1}f", $number - 0.5 * 0.1 ** $digits);
-
-            return round((float) $result, $digits, PHP_ROUND_HALF_DOWN);
+        if (PHP_VERSION_ID >= 80400) {
+            return round(
+                (float) (string) $number,
+                $digits,
+                RoundingMode::AwayFromZero //* @phpstan-ignore-line
+            );
         }
 
-        if ($digitsPlus1 < 0) {
-            return round($number + 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_DOWN);
+        if ($number < 0.0) {
+            return round($number - 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_DOWN);
         }
-        $result = sprintf("%.{$digitsPlus1}f", $number + 0.5 * 0.1 ** $digits);
 
-        return round((float) $result, $digits, PHP_ROUND_HALF_DOWN);
+        return round($number + 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_DOWN);
     }
 
     /**
      * ROUNDDOWN.
      *
      * Rounds a number down to a specified number of decimal places
      *
-     * @param array|float $number Number to round, or can be an array of numbers
-     * @param array|int $digits Number of digits to which you want to round $number, or can be an array of numbers
+     * @param null|array|float|string $number Number to round, or can be an array of numbers
+     * @param array|float|int|string $digits Number of digits to which you want to round $number, or can be an array of numbers
      *
      * @return array|float|string Rounded Number, or a string containing an error
      *         If an array of numbers is passed as the argument, then the returned result will also be an array
@@ -114,23 +113,19 @@ public static function down($number, $digits)
             return 0.0;
         }
 
-        $digitsPlus1 = $digits + 1;
-        if ($number < 0.0) {
-            if ($digitsPlus1 < 0) {
-                return round($number + 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_UP);
-            }
-            $result = sprintf("%.{$digitsPlus1}f", $number + 0.5 * 0.1 ** $digits);
-
-            return round((float) $result, $digits, PHP_ROUND_HALF_UP);
+        if (PHP_VERSION_ID >= 80400) {
+            return round(
+                (float) (string) $number,
+                $digits,
+                RoundingMode::TowardsZero //* @phpstan-ignore-line
+            );
         }
 
-        if ($digitsPlus1 < 0) {
-            return round($number - 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_UP);
+        if ($number < 0.0) {
+            return round($number + 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_UP);
         }
 
-        $result = sprintf("%.{$digitsPlus1}f", $number - 0.5 * 0.1 ** $digits);
-
-        return round((float) $result, $digits, PHP_ROUND_HALF_UP);
+        return round($number - 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_UP);
     }
 
     /**
@@ -141,7 +136,7 @@ public static function down($number, $digits)
      * @param mixed $number Expect float. Number to round, or can be an array of numbers
      * @param mixed $multiple Expect int. Multiple to which you want to round, or can be an array of numbers.
      *
-     * @return array|float|string Rounded Number, or a string containing an error
+     * @return array|float|int|string Rounded Number, or a string containing an error
      *         If an array of numbers is passed as the argument, then the returned result will also be an array
      *            with the same dimensions
      */
@@ -210,7 +205,7 @@ public static function even($number)
      *
      * @param array|float $number Number to round, or can be an array of numbers
      *
-     * @return array|float|string Rounded Number, or a string containing an error
+     * @return array|float|int|string Rounded Number, or a string containing an error
      *         If an array of numbers is passed as the argument, then the returned result will also be an array
      *            with the same dimensions
      */

diff --git a/src/PhpSpreadsheet/Calculation/MathTrig/Trunc.php b/src/PhpSpreadsheet/Calculation/MathTrig/Trunc.php
@@ -3,7 +3,6 @@
 namespace PhpOffice\PhpSpreadsheet\Calculation\MathTrig;
 
 use PhpOffice\PhpSpreadsheet\Calculation\ArrayEnabled;
-use PhpOffice\PhpSpreadsheet\Calculation\Exception;
 
 class Trunc
 {
@@ -13,11 +12,14 @@ class Trunc
      * TRUNC.
      *
      * Truncates value to the number of fractional digits by number_digits.
+     * This will probably not be the precise result in the unlikely
+     * event that the number of digits to the left of the decimal
+     * plus the number of digits to the right exceeds PHP_FLOAT_DIG
+     * (or possibly that value minus 1).
+     * Excel is unlikely to do any better.
      *
-     * @param array|float $value
-     *                      Or can be an array of values
-     * @param array|int $digits
-     *                      Or can be an array of values
+     * @param null|array|float|string $value Or can be an array of values
+     * @param array|float|int|string $digits Or can be an array of values
      *
      * @return array|float|string Truncated value, or a string containing an error
      *         If an array of numbers is passed as an argument, then the returned result will also be an array
@@ -29,33 +31,6 @@ public static function evaluate($value = 0, $digits = 0)
             return self::evaluateArrayArguments([self::class, __FUNCTION__], $value, $digits);
         }
 
-        try {
-            $value = Helpers::validateNumericNullBool($value);
-            $digits = Helpers::validateNumericNullSubstitution($digits, null);
-        } catch (Exception $e) {
-            return $e->getMessage();
-        }
-
-        if ($value == 0) {
-            return $value;
-        }
-
-        if ($value >= 0) {
-            $minusSign = '';
-        } else {
-            $minusSign = '-';
-            $value = -$value;
-        }
-
-        $digits = (int) floor($digits);
-        if ($digits < 0) {
-            $power = (int) (10 ** -$digits);
-            $result = intdiv((int) floor($value), $power) * $power;
-            return ($minusSign === '') ? $result : -$result;
-        }
-        $digitsPlus1 = $digits + 1;
-        $result = substr($minusSign . sprintf("%.{$digitsPlus1}f", $value), 0, -1);
-
-        return (float) $result;
+        return Round::down($value, $digits);
     }
 }
diff --git a/src/PhpSpreadsheet/Reader/Security/XmlScanner.php b/src/PhpSpreadsheet/Reader/Security/XmlScanner.php
@@ -6,6 +6,9 @@
 
 class XmlScanner
 {
+    private const ENCODING_PATTERN = '/encoding\\s*=\\s*(["\'])(.+?)\\1/s';
+    private const ENCODING_UTF7 = '/encoding\\s*=\\s*(["\'])UTF-7\\1/si';
+
     /**
      * String used to identify risky xml elements.
      *
@@ -114,29 +117,41 @@ private static function forceString($arg): string
     private function toUtf8($xml)
     {
         $charset = $this->findCharSet($xml);
+        $foundUtf7 = $charset === 'UTF-7';
         if ($charset !== 'UTF-8') {
+            $testStart = '/^.{0,4}\\s*<?xml/s';
+            $startWithXml1 = preg_match($testStart, $xml);
             $xml = self::forceString(mb_convert_encoding($xml, 'UTF-8', $charset));
-
-            $charset = $this->findCharSet($xml);
-            if ($charset !== 'UTF-8') {
-                throw new Reader\Exception('Suspicious Double-encoded XML, spreadsheet file load() aborted to prevent XXE/XEE attacks');
+            if ($startWithXml1 === 1 && preg_match($testStart, $xml) !== 1) {
+                throw new Reader\Exception('Double encoding not permitted');
             }
+            $foundUtf7 = $foundUtf7 || (preg_match(self::ENCODING_UTF7, $xml) === 1);
+            $xml = preg_replace(self::ENCODING_PATTERN, '', $xml) ?? $xml;
+        } else {
+            $foundUtf7 = $foundUtf7 || (preg_match(self::ENCODING_UTF7, $xml) === 1);
+        }
+        if ($foundUtf7) {
+            throw new Reader\Exception('UTF-7 encoding not permitted');
+        }
+        if (substr($xml, 0, Reader\Csv::UTF8_BOM_LEN) === Reader\Csv::UTF8_BOM) {
+            $xml = substr($xml, Reader\Csv::UTF8_BOM_LEN);
         }
 
         return $xml;
     }
 
     private function findCharSet(string $xml): string
     {
-        $patterns = [
-            '/encoding\\s*=\\s*"([^"]*]?)"/',
-            "/encoding\\s*=\\s*'([^']*?)'/",
-        ];
-
-        foreach ($patterns as $pattern) {
-            if (preg_match($pattern, $xml, $matches)) {
-                return strtoupper($matches[1]);
-            }
+        if (substr($xml, 0, 4) === "\x4c\x6f\xa7\x94") {
+            throw new Reader\Exception('EBCDIC encoding not permitted');
+        }
+        $encoding = Reader\Csv::guessEncodingBom('', $xml);
+        if ($encoding !== '') {
+            return $encoding;
+        }
+        $xml = str_replace("\0", '', $xml);
+        if (preg_match(self::ENCODING_PATTERN, $xml, $matches)) {
+            return strtoupper($matches[2]);
         }
 
         return 'UTF-8';
@@ -151,13 +166,16 @@ private function findCharSet(string $xml): string
      */
     public function scan($xml)
     {
-        $xml = "$xml";
         $this->disableEntityLoaderCheck();
+        // Don't rely purely on libxml_disable_entity_loader()
+        $pattern = '/\\0*' . implode('\\0*', /** @scrutinizer ignore-type */ str_split($this->pattern)) . '\\0*/';
 
-        $xml = $this->toUtf8($xml);
+        $xml = "$xml";
+        if (preg_match($pattern, $xml)) {
+            throw new Reader\Exception('Detected use of ENTITY in XML, spreadsheet file load() aborted to prevent XXE/XEE attacks');
+        }
 
-        // Don't rely purely on libxml_disable_entity_loader()
-        $pattern = '/\\0?' . implode('\\0?', /** @scrutinizer ignore-type */ str_split($this->pattern)) . '\\0?/';
+        $xml = $this->toUtf8($xml);
 
         if (preg_match($pattern, $xml)) {
             throw new Reader\Exception('Detected use of ENTITY in XML, spreadsheet file load() aborted to prevent XXE/XEE attacks');
@@ -171,7 +189,7 @@ public function scan($xml)
     }
 
     /**
-     * Scan theXML for use of <!ENTITY to prevent XXE/XEE attacks.
+     * Scan the XML for use of <!ENTITY to prevent XXE/XEE attacks.
      *
      * @param string $filestream
      *