Skip to content

Commit 08d4e08

Browse files
oleibmanoleibman
committed
Backport Security Fix
1 parent 8628d63 commit 08d4e08

File tree

18 files changed

+210
-102
lines changed

18 files changed

+210
-102
lines changed

‎.github/workflows/main.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ jobs:
1313
- '8.1'
1414
- '8.2'
1515
- '8.3'
16+
- '8.4'
1617

1718
include:
1819
- php-version: 'nightly'

‎CHANGELOG.md‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,17 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com)
66
and this project adheres to [Semantic Versioning](https://semver.org).
77

8-
## TBD - 2.3.1
8+
## 2024-11-10 - 2.3.1
9+
10+
### Fixed
11+
12+
- Backported security patches.
13+
- Write ignoredErrors Tag Before Drawings. Backport of [PR #4212](https://github.com/PHPOffice/PhpSpreadsheet/pull/4212) intended for 3.4.0.
14+
- Changes to ROUNDDOWN/ROUNDUP/TRUNC. Backport of [PR #4214](https://github.com/PHPOffice/PhpSpreadsheet/pull/4214) intended for 3.4.0.
915

1016
### Added
1117

12-
- Method to Test Whether Csv Will Be Affected by Php9 (backport of PR #4189 intended for 3.4.0).
18+
- 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.
1319

1420
## 2024-09-29 - 2.3.0
1521

‎src/PhpSpreadsheet/Calculation/MathTrig/Round.php‎

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
use PhpOffice\PhpSpreadsheet\Calculation\ArrayEnabled;
66
use PhpOffice\PhpSpreadsheet\Calculation\Exception;
77
use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
8+
// following added in Php8.4
9+
use RoundingMode;
810

911
class Round
1012
{
@@ -67,31 +69,28 @@ public static function up($number, $digits): array|string|float
6769
return 0.0;
6870
}
6971

70-
$digitsPlus1 = $digits + 1;
71-
if ($number < 0.0) {
72-
if ($digitsPlus1 < 0) {
73-
return round($number - 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_DOWN);
74-
}
75-
$result = sprintf("%.{$digitsPlus1}F", $number - 0.5 * 0.1 ** $digits);
76-
77-
return round((float) $result, $digits, PHP_ROUND_HALF_DOWN);
72+
if (PHP_VERSION_ID >= 80400) {
73+
return round(
74+
(float) (string) $number,
75+
$digits,
76+
RoundingMode::AwayFromZero //* @phpstan-ignore-line
77+
);
7878
}
7979

80-
if ($digitsPlus1 < 0) {
81-
return round($number + 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_DOWN);
80+
if ($number < 0.0) {
81+
return round($number - 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_DOWN);
8282
}
83-
$result = sprintf("%.{$digitsPlus1}F", $number + 0.5 * 0.1 ** $digits);
8483

85-
return round((float) $result, $digits, PHP_ROUND_HALF_DOWN);
84+
return round($number + 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_DOWN);
8685
}
8786

8887
/**
8988
* ROUNDDOWN.
9089
*
9190
* Rounds a number down to a specified number of decimal places
9291
*
93-
* @param array|float $number Number to round, or can be an array of numbers
94-
* @param array|int $digits Number of digits to which you want to round $number, or can be an array of numbers
92+
* @param null|array|float|string $number Number to round, or can be an array of numbers
93+
* @param array|float|int|string $digits Number of digits to which you want to round $number, or can be an array of numbers
9594
*
9695
* @return array|float|string Rounded Number, or a string containing an error
9796
* 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): array|string|float
114113
return 0.0;
115114
}
116115

117-
$digitsPlus1 = $digits + 1;
118-
if ($number < 0.0) {
119-
if ($digitsPlus1 < 0) {
120-
return round($number + 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_UP);
121-
}
122-
$result = sprintf("%.{$digitsPlus1}F", $number + 0.5 * 0.1 ** $digits);
123-
124-
return round((float) $result, $digits, PHP_ROUND_HALF_UP);
116+
if (PHP_VERSION_ID >= 80400) {
117+
return round(
118+
(float) (string) $number,
119+
$digits,
120+
RoundingMode::TowardsZero //* @phpstan-ignore-line
121+
);
125122
}
126123

127-
if ($digitsPlus1 < 0) {
128-
return round($number - 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_UP);
124+
if ($number < 0.0) {
125+
return round($number + 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_UP);
129126
}
130127

131-
$result = sprintf("%.{$digitsPlus1}F", $number - 0.5 * 0.1 ** $digits);
132-
133-
return round((float) $result, $digits, PHP_ROUND_HALF_UP);
128+
return round($number - 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_UP);
134129
}
135130

136131
/**

‎src/PhpSpreadsheet/Calculation/MathTrig/Trunc.php‎

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
namespace PhpOffice\PhpSpreadsheet\Calculation\MathTrig;
44

55
use PhpOffice\PhpSpreadsheet\Calculation\ArrayEnabled;
6-
use PhpOffice\PhpSpreadsheet\Calculation\Exception;
76

87
class Trunc
98
{
@@ -19,47 +18,19 @@ class Trunc
1918
* (or possibly that value minus 1).
2019
* Excel is unlikely to do any better.
2120
*
22-
* @param array|float $value Or can be an array of values
23-
* @param array|int $digits Or can be an array of values
21+
* @param null|array|float|string $value Or can be an array of values
22+
* @param array|float|int|string $digits Or can be an array of values
2423
*
2524
* @return array|float|string Truncated value, or a string containing an error
2625
* If an array of numbers is passed as an argument, then the returned result will also be an array
2726
* with the same dimensions
2827
*/
29-
public static function evaluate(array|float|string|null $value = 0, array|int|string $digits = 0): array|float|string
28+
public static function evaluate(array|float|string|null $value = 0, array|float|int|string $digits = 0): array|float|string
3029
{
3130
if (is_array($value) || is_array($digits)) {
3231
return self::evaluateArrayArguments([self::class, __FUNCTION__], $value, $digits);
3332
}
3433

35-
try {
36-
$value = Helpers::validateNumericNullBool($value);
37-
$digits = Helpers::validateNumericNullSubstitution($digits, null);
38-
} catch (Exception $e) {
39-
return $e->getMessage();
40-
}
41-
42-
if ($value == 0) {
43-
return $value;
44-
}
45-
46-
if ($value >= 0) {
47-
$minusSign = '';
48-
} else {
49-
$minusSign = '-';
50-
$value = -$value;
51-
}
52-
$digits = (int) floor($digits);
53-
if ($digits < 0) {
54-
$result = (float) (substr(sprintf('%.0F', $value), 0, $digits) . str_repeat('0', -$digits));
55-
56-
return ($minusSign === '') ? $result : -$result;
57-
}
58-
$decimals = (floor($value) == (int) $value) ? (PHP_FLOAT_DIG - strlen((string) (int) $value)) : $digits;
59-
$resultString = ($decimals < 0) ? sprintf('%F', $value) : sprintf('%.' . $decimals . 'F', $value);
60-
$regExp = '/([.]\\d{' . $digits . '})\\d+$/';
61-
$result = $minusSign . (preg_replace($regExp, '$1', $resultString) ?? $resultString);
62-
63-
return (float) $result;
34+
return Round::down($value, $digits);
6435
}
6536
}

‎src/PhpSpreadsheet/Reader/Security/XmlScanner.php‎

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66

77
class XmlScanner
88
{
9+
private const ENCODING_PATTERN = '/encoding\\s*=\\s*(["\'])(.+?)\\1/s';
10+
private const ENCODING_UTF7 = '/encoding\\s*=\\s*(["\'])UTF-7\\1/si';
11+
912
private string $pattern;
1013

1114
/** @var ?callable */
@@ -36,29 +39,41 @@ private static function forceString(mixed $arg): string
3639
private function toUtf8(string $xml): string
3740
{
3841
$charset = $this->findCharSet($xml);
42+
$foundUtf7 = $charset === 'UTF-7';
3943
if ($charset !== 'UTF-8') {
44+
$testStart = '/^.{0,4}\\s*<?xml/s';
45+
$startWithXml1 = preg_match($testStart, $xml);
4046
$xml = self::forceString(mb_convert_encoding($xml, 'UTF-8', $charset));
41-
42-
$charset = $this->findCharSet($xml);
43-
if ($charset !== 'UTF-8') {
44-
throw new Reader\Exception('Suspicious Double-encoded XML, spreadsheet file load() aborted to prevent XXE/XEE attacks');
47+
if ($startWithXml1 === 1 && preg_match($testStart, $xml) !== 1) {
48+
throw new Reader\Exception('Double encoding not permitted');
4549
}
50+
$foundUtf7 = $foundUtf7 || (preg_match(self::ENCODING_UTF7, $xml) === 1);
51+
$xml = preg_replace(self::ENCODING_PATTERN, '', $xml) ?? $xml;
52+
} else {
53+
$foundUtf7 = $foundUtf7 || (preg_match(self::ENCODING_UTF7, $xml) === 1);
54+
}
55+
if ($foundUtf7) {
56+
throw new Reader\Exception('UTF-7 encoding not permitted');
57+
}
58+
if (substr($xml, 0, Reader\Csv::UTF8_BOM_LEN) === Reader\Csv::UTF8_BOM) {
59+
$xml = substr($xml, Reader\Csv::UTF8_BOM_LEN);
4660
}
4761

4862
return $xml;
4963
}
5064

5165
private function findCharSet(string $xml): string
5266
{
53-
$patterns = [
54-
'/encoding\\s*=\\s*"([^"]*]?)"/',
55-
"/encoding\\s*=\\s*'([^']*?)'/",
56-
];
57-
58-
foreach ($patterns as $pattern) {
59-
if (preg_match($pattern, $xml, $matches)) {
60-
return strtoupper($matches[1]);
61-
}
67+
if (substr($xml, 0, 4) === "\x4c\x6f\xa7\x94") {
68+
throw new Reader\Exception('EBCDIC encoding not permitted');
69+
}
70+
$encoding = Reader\Csv::guessEncodingBom('', $xml);
71+
if ($encoding !== '') {
72+
return $encoding;
73+
}
74+
$xml = str_replace("\0", '', $xml);
75+
if (preg_match(self::ENCODING_PATTERN, $xml, $matches)) {
76+
return strtoupper($matches[2]);
6277
}
6378

6479
return 'UTF-8';
@@ -71,13 +86,16 @@ private function findCharSet(string $xml): string
7186
*/
7287
public function scan($xml): string
7388
{
89+
// Don't rely purely on libxml_disable_entity_loader()
90+
$pattern = '/\\0*' . implode('\\0*', str_split($this->pattern)) . '\\0*/';
91+
7492
$xml = "$xml";
93+
if (preg_match($pattern, $xml)) {
94+
throw new Reader\Exception('Detected use of ENTITY in XML, spreadsheet file load() aborted to prevent XXE/XEE attacks');
95+
}
7596

7697
$xml = $this->toUtf8($xml);
7798

78-
// Don't rely purely on libxml_disable_entity_loader()
79-
$pattern = '/\\0?' . implode('\\0?', str_split($this->pattern)) . '\\0?/';
80-
8199
if (preg_match($pattern, $xml)) {
82100
throw new Reader\Exception('Detected use of ENTITY in XML, spreadsheet file load() aborted to prevent XXE/XEE attacks');
83101
}
@@ -90,7 +108,7 @@ public function scan($xml): string
90108
}
91109

92110
/**
93-
* Scan theXML for use of <!ENTITY to prevent XXE/XEE attacks.
111+
* Scan the XML for use of <!ENTITY to prevent XXE/XEE attacks.
94112
*/
95113
public function scanFile(string $filestream): string
96114
{

‎src/PhpSpreadsheet/Reader/Xml.php‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
use PhpOffice\PhpSpreadsheet\Settings;
1919
use PhpOffice\PhpSpreadsheet\Shared\Date;
2020
use PhpOffice\PhpSpreadsheet\Shared\File;
21-
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
2221
use PhpOffice\PhpSpreadsheet\Spreadsheet;
2322
use PhpOffice\PhpSpreadsheet\Worksheet\SheetView;
2423
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
@@ -79,7 +78,8 @@ public function canRead(string $filename): bool
7978
];
8079

8180
// Open file
82-
$data = file_get_contents($filename) ?: '';
81+
$data = (string) file_get_contents($filename);
82+
$data = $this->getSecurityScannerOrThrow()->scan($data);
8383

8484
// Why?
8585
//$data = str_replace("'", '"', $data); // fix headers with single quote
@@ -93,15 +93,6 @@ public function canRead(string $filename): bool
9393
break;
9494
}
9595
}
96-
97-
// Retrieve charset encoding
98-
if (preg_match('/<?xml.*encoding=[\'"](.*?)[\'"].*?>/m', $data, $matches)) {
99-
$charSet = strtoupper($matches[1]);
100-
if (preg_match('/^ISO-8859-\d[\dL]?$/i', $charSet) === 1) {
101-
$data = StringHelper::convertEncoding($data, 'UTF-8', $charSet);
102-
$data = (string) preg_replace('/(<?xml.*encoding=[\'"]).*?([\'"].*?>)/um', '$1' . 'UTF-8' . '$2', $data, 1);
103-
}
104-
}
10596
$this->fileContents = $data;
10697

10798
return $valid;

‎src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ public function writeWorksheet(PhpspreadsheetWorksheet $worksheet, array $string
123123
// Breaks
124124
$this->writeBreaks($objWriter, $worksheet);
125125

126+
// IgnoredErrors
127+
$this->writeIgnoredErrors($objWriter);
128+
126129
// Drawings and/or Charts
127130
$this->writeDrawings($objWriter, $worksheet, $includeCharts);
128131

@@ -135,9 +138,6 @@ public function writeWorksheet(PhpspreadsheetWorksheet $worksheet, array $string
135138
// AlternateContent
136139
$this->writeAlternateContent($objWriter, $worksheet);
137140

138-
// IgnoredErrors
139-
$this->writeIgnoredErrors($objWriter);
140-
141141
// BackgroundImage must come after ignored, before table
142142
$this->writeBackgroundImage($objWriter, $worksheet);
143143

‎tests/PhpSpreadsheetTests/Reader/Security/XmlScannerTest.php‎

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,11 @@ public static function providerValidXML(): array
3030
self::assertNotFalse($glob);
3131
foreach ($glob as $file) {
3232
$filename = realpath($file);
33-
$expectedResult = file_get_contents($file);
33+
$expectedResult = (string) file_get_contents($file);
34+
if (preg_match('/UTF-16(LE|BE)?/', $file, $matches) == 1) {
35+
$expectedResult = (string) mb_convert_encoding($expectedResult, 'UTF-8', $matches[0]);
36+
$expectedResult = preg_replace('/encoding\\s*=\\s*[\'"]UTF-\\d+(LE|BE)?[\'"]/', '', $expectedResult) ?? $expectedResult;
37+
}
3438
$tests[basename($file)] = [$filename, $expectedResult];
3539
}
3640

@@ -132,19 +136,47 @@ public function testEncodingAllowsMixedCase(): void
132136
self::assertSame($input, $output);
133137
}
134138

135-
public function testUtf7Whitespace(): void
139+
/**
140+
* @dataProvider providerInvalidXlsx
141+
*/
142+
public function testInvalidXlsx(string $filename, string $message): void
136143
{
137144
$this->expectException(ReaderException::class);
138-
$this->expectExceptionMessage('Double-encoded');
145+
$this->expectExceptionMessage($message);
139146
$reader = new Xlsx();
140-
$reader->load('tests/data/Reader/XLSX/utf7white.dontuse');
147+
$reader->load("tests/data/Reader/XLSX/$filename");
141148
}
142149

143-
public function testUtf8Entity(): void
150+
public static function providerInvalidXlsx(): array
151+
{
152+
return [
153+
['utf7white.dontuse', 'UTF-7 encoding not permitted'],
154+
['utf7quoteorder.dontuse', 'UTF-7 encoding not permitted'],
155+
['utf8and16.dontuse', 'Double encoding not permitted'],
156+
['utf8and16.entity.dontuse', 'Detected use of ENTITY'],
157+
['utf8entity.dontuse', 'Detected use of ENTITY'],
158+
['utf16entity.dontuse', 'Detected use of ENTITY'],
159+
['ebcdic.dontuse', 'EBCDIC encoding not permitted'],
160+
];
161+
}
162+
163+
/**
164+
* @dataProvider providerValidUtf16
165+
*/
166+
public function testValidUtf16(string $filename): void
144167
{
145-
$this->expectException(ReaderException::class);
146-
$this->expectExceptionMessage('Detected use of ENTITY');
147168
$reader = new Xlsx();
148-
$reader->load('tests/data/Reader/XLSX/utf8entity.dontuse');
169+
$spreadsheet = $reader->load("tests/data/Reader/XLSX/$filename");
170+
$sheet = $spreadsheet->getActiveSheet();
171+
self::assertSame(1, $sheet->getCell('A1')->getValue());
172+
$spreadsheet->disconnectWorksheets();
173+
}
174+
175+
public static function providerValidUtf16(): array
176+
{
177+
return [
178+
['utf16be.xlsx'],
179+
['utf16be.bom.xlsx'],
180+
];
149181
}
150182
}

0 commit comments

Comments
 (0)