diff --git a/src/PhpSpreadsheet/Reader/Security/XmlScanner.php b/src/PhpSpreadsheet/Reader/Security/XmlScanner.php index 7b4e10431..5d238a80b 100644 --- a/src/PhpSpreadsheet/Reader/Security/XmlScanner.php +++ b/src/PhpSpreadsheet/Reader/Security/XmlScanner.php @@ -56,6 +56,11 @@ class XmlScanner throw new Reader\Exception('UTF-7 encoding not permitted'); } if (substr($xml, 0, Reader\Csv::UTF8_BOM_LEN) === Reader\Csv::UTF8_BOM) { + if (preg_match(self::ENCODING_PATTERN, $xml, $matches) === 1) { + if (strtolower($matches[2]) !== 'utf-8') { + throw new Reader\Exception("BOM says UTF-8 but encoding says {$matches[2]}"); + } + } $xml = substr($xml, Reader\Csv::UTF8_BOM_LEN); } diff --git a/src/PhpSpreadsheet/Reader/Xml.php b/src/PhpSpreadsheet/Reader/Xml.php index f5cdc8108..9e5bd9975 100644 --- a/src/PhpSpreadsheet/Reader/Xml.php +++ b/src/PhpSpreadsheet/Reader/Xml.php @@ -2,6 +2,7 @@ namespace PhpOffice\PhpSpreadsheet\Reader; +use Composer\Pcre\Preg; use DateTime; use DateTimeZone; use PhpOffice\PhpSpreadsheet\Cell\AddressHelper; @@ -53,9 +54,14 @@ class Xml extends BaseReader public static function unentity(string $contents): string { - $contents = preg_replace('/&(amp|lt|gt|quot|apos);/', "\u{fffe}\u{feff}\$1;", trim($contents)) ?? $contents; + // fffe is invalid, replace with replacement char + $contents = str_replace("\u{fffe}", "\u{fffd}", $contents); + // use positive lookahead to "protect" valid xml entities + $contents = Preg::replace('/&(?=(?:amp|lt|gt|quot|apos|#[0-9]+|#x[0-9a-fA-F]+);)/', "\u{fffe}", trim($contents)); + // now decode remaining html entities $contents = html_entity_decode($contents, ENT_NOQUOTES | ENT_SUBSTITUTE | ENT_HTML401, 'UTF-8'); - $contents = str_replace("\u{fffe}\u{feff}", '&', $contents); + // Escape remaining ampersands, restore those which were replaced with fffe + $contents = str_replace(['&', "\u{fffe}"], ['&', '&'], $contents); return $contents; } @@ -651,7 +657,7 @@ class Xml extends BaseReader } $rangeCalculated = false; if (isset($xmlX->WorksheetOptions->Panes->Pane->RangeSelection)) { - if (1 === preg_match('/^R(\d+)C(\d+):R(\d+)C(\d+)$/', (string) $xmlX->WorksheetOptions->Panes->Pane->RangeSelection, $selectionMatches)) { + if (Preg::isMatch('/^R(\d+)C(\d+):R(\d+)C(\d+)$/', (string) $xmlX->WorksheetOptions->Panes->Pane->RangeSelection, $selectionMatches)) { $selectedCell = Coordinate::stringFromColumnIndex((int) $selectionMatches[2]) . $selectionMatches[1] . ':' diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/Sec973cTest.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/Sec973cTest.php new file mode 100644 index 000000000..1b29cb968 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Xlsx/Sec973cTest.php @@ -0,0 +1,20 @@ +expectException(ReaderException::class); + $this->expectExceptionMessage('BOM says UTF-8 but encoding says ISO-2022-JP'); + $reader = new XlsxReader(); + $reader->load('tests/data/Reader/XLSX/sec937c.xlsx'); + } +} diff --git a/tests/PhpSpreadsheetTests/Reader/Xml/HtmlEntitiesLoadTest.php b/tests/PhpSpreadsheetTests/Reader/Xml/HtmlEntitiesLoadTest.php index 197e00841..60348d86c 100644 --- a/tests/PhpSpreadsheetTests/Reader/Xml/HtmlEntitiesLoadTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Xml/HtmlEntitiesLoadTest.php @@ -9,7 +9,7 @@ use PHPUnit\Framework\TestCase; class HtmlEntitiesLoadTest extends TestCase { - public static function testIssue2157(): void + public function testIssue2157(): void { $infile = 'tests/data/Reader/Xml/issue.2157.small.xml'; $contents = (string) file_get_contents($infile); @@ -26,4 +26,12 @@ class HtmlEntitiesLoadTest extends TestCase self::assertStringContainsString('
', $g2); $spreadsheet->disconnectWorksheets(); } + + public function testUnknownEntities(): void + { + $string = '& &Amp; Τ  㼒 g12; & &*3 <'; + $expected = '& &Amp; Τ  㼒 &#x3g12; & &*3 <'; + $result = XmlReader::unentity($string); + self::assertSame($expected, $result); + } } diff --git a/tests/PhpSpreadsheetTests/Reader/Xml/Sec25mgTest.php b/tests/PhpSpreadsheetTests/Reader/Xml/Sec25mgTest.php new file mode 100644 index 000000000..345b41050 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Xml/Sec25mgTest.php @@ -0,0 +1,132 @@ +filename !== '') { + unlink($this->filename); + } + } + + public function testNumericEntities(): void + { + $this->filename = File::temporaryFilename(); + $entity_value = str_repeat('A', 100000); + $refs = str_repeat('&big;', 200); + + $xml = << + + &#60;!DOCTYPE Workbook [ + &#60;!ENTITY big "$entity_value"> + ]> + + + + $refs +
+
+
+ EOF; + self::assertNotFalse( + file_put_contents($this->filename, $xml) + ); + $this->expectException(ReaderException::class); + $this->expectExceptionMessage('Cannot load invalid XML file'); + $reader = new XmlReader(); + $reader->load($this->filename); + } + + public function testDetectDoctype(): void + { + $this->filename = File::temporaryFilename(); + $entity_value = str_repeat('A', 100000); + $refs = str_repeat('&big;', 200); + + $xml = << + + + ]> + + + + $refs +
+
+
+ EOF; + self::assertNotFalse( + file_put_contents($this->filename, $xml) + ); + $this->expectException(ReaderException::class); + $this->expectExceptionMessage('Detected use of ENTITY'); + $reader = new XmlReader(); + $reader->load($this->filename); + } + + public function testNoDoctype(): void + { + $this->filename = File::temporaryFilename(); + $refs = str_repeat('&πϏ0', 200); + $refsOut = str_repeat('&πϏ0', 200); + + $xml = << + + + + + $refs +
+
+
+ EOF; + self::assertNotFalse( + file_put_contents($this->filename, $xml) + ); + $reader = new XmlReader(); + $spreadsheet = $reader->load($this->filename); + $sheet = $spreadsheet->getActiveSheet(); + self::assertSame($refsOut, $sheet->getCell('A1')->getValue()); + $spreadsheet->disconnectWorksheets(); + } + + public function testIsolated(): void + { + $scanner = new XmlScanner('setAdditionalCallback([XmlReader::class, 'unentity']); + $input = '&#60;!DOCTYPE test'; + $pass1 = $scanner->scan($input); + self::assertStringNotContainsString('scan($pass1); + self::assertStringNotContainsString('