diff --git a/src/PhpSpreadsheet/Reader/Ods.php b/src/PhpSpreadsheet/Reader/Ods.php index 08fb4f19a..bb4b7b1ef 100644 --- a/src/PhpSpreadsheet/Reader/Ods.php +++ b/src/PhpSpreadsheet/Reader/Ods.php @@ -214,6 +214,7 @@ class Ods extends BaseReader if ($xml->name == 'table:table-row' && $xml->nodeType == XMLReader::ELEMENT) { $rowspan = $xml->getAttribute('table:number-rows-repeated'); $rowspan = empty($rowspan) ? 1 : (int) $rowspan; + self::checkRowsRepeated($currRow, $rowspan); $currRow += $rowspan; $currCol = 0; // Step into the row @@ -223,6 +224,10 @@ class Ods extends BaseReader if ($xml->name == 'table:table-cell' && $xml->nodeType == XMLReader::ELEMENT) { $mergeSize = $xml->getAttribute('table:number-columns-repeated'); $mergeSize = empty($mergeSize) ? 1 : (int) $mergeSize; + self::checkColumnsRepeatedInt( + $currCol, + $mergeSize + ); $currCol += $mergeSize; if (!$xml->isEmptyElement) { $tmpInfo['totalColumns'] = max($tmpInfo['totalColumns'], $currCol); @@ -232,6 +237,11 @@ class Ods extends BaseReader } } elseif ($xml->name == 'table:covered-table-cell' && $xml->nodeType == XMLReader::ELEMENT) { $mergeSize = $xml->getAttribute('table:number-columns-repeated'); + $mergeSize = empty($mergeSize) ? 1 : (int) $mergeSize; + self::checkColumnsRepeatedInt( + $currCol, + $mergeSize + ); $currCol += (int) $mergeSize; } if ($doread) { @@ -328,7 +338,7 @@ class Ods extends BaseReader ->scan($zip->getFromName('meta.xml')) ); if ($xml === false) { - throw new Exception('Unable to read data from {$pFilename}'); + throw new Exception("Unable to read data from {$filename}"); } /** @var array{meta?: string, office?: string, dc?: string} */ @@ -815,6 +825,7 @@ class Ods extends BaseReader } else { $rowRepeats = 1; } + self::checkRowsRepeated($rowID, $rowRepeats); $worksheet = $spreadsheet->getSheetByName($worksheetName); $columnID = 'A'; @@ -828,6 +839,8 @@ class Ods extends BaseReader } else { $colRepeats = 1; } + $columnIndex = Coordinate::columnIndexFromString($columnID); + self::checkColumnsRepeated($columnID, $colRepeats); $styleName = $cellData->getAttributeNS($tableNs, 'style-name'); if ($styleName === '') { if ($worksheet === null || !$worksheet->columnDimensionExists($columnID)) { @@ -1373,17 +1386,19 @@ class Ods extends BaseReader bool $processStyles = true ): void { if ($childNode->hasAttributeNS($tableNs, 'number-columns-repeated')) { - $rowRepeats = (int) $childNode->getAttributeNS($tableNs, 'number-columns-repeated'); + $colRepeats = (int) $childNode->getAttributeNS($tableNs, 'number-columns-repeated'); } else { - $rowRepeats = 1; + $colRepeats = 1; } + // called routine expects index to be 1 less than it is + self::checkColumnsRepeatedInt($tableColumnIndex - 1, $colRepeats); $tableStyleName = $childNode->getAttributeNS($tableNs, 'style-name'); if ($processWidths) { if (isset($columnWidths[$tableStyleName])) { $columnWidth = new HelperDimension($columnWidths[$tableStyleName]); $tableColumnIndex2 = $tableColumnIndex; $tableColumnString = Coordinate::stringFromColumnIndex($tableColumnIndex2); - for ($rowRepeats2 = $rowRepeats; $rowRepeats2 > 0 && $tableColumnIndex2 <= AddressRange::MAX_COLUMN_INT; --$rowRepeats2) { + for ($colRepeats2 = $colRepeats; $colRepeats2 > 0 && $tableColumnIndex2 <= AddressRange::MAX_COLUMN_INT; --$colRepeats2) { if (!$this->readEmptyCells && $tableColumnIndex2 > $this->highestDataIndex) { break; } @@ -1402,7 +1417,7 @@ class Ods extends BaseReader if ($defaultStyleName !== 'Default' && isset($this->allStyles[$defaultStyleName])) { $tableColumnIndex2 = $tableColumnIndex; $tableColumnString = Coordinate::stringFromColumnIndex($tableColumnIndex2); - for ($rowRepeats2 = $rowRepeats; $rowRepeats2 > 0 && $tableColumnIndex2 <= AddressRange::MAX_COLUMN_INT; --$rowRepeats2) { + for ($colRepeats2 = $colRepeats; $colRepeats2 > 0 && $tableColumnIndex2 <= AddressRange::MAX_COLUMN_INT; --$colRepeats2) { $spreadsheet->getActiveSheet() ->getStyle($tableColumnString) ->applyFromArray( @@ -1415,7 +1430,7 @@ class Ods extends BaseReader } } } - $tableColumnIndex += $rowRepeats; + $tableColumnIndex += $colRepeats; } private function processSettings(ZipArchive $zip, Spreadsheet $spreadsheet): void @@ -1858,6 +1873,30 @@ class Ods extends BaseReader } } + private static function checkRowsRepeated(int $rowID, int $rowRepeats): void + { + if ($rowRepeats < 1 || $rowID + $rowRepeats - 1 > AddressRange::MAX_ROW) { + throw new Exception("Invalid number-rows-repeated $rowRepeats following row $rowID"); + } + } + + private static function checkColumnsRepeated(string $colID, int $colRepeats): void + { + $colIndex = Coordinate::columnIndexFromString($colID); + if ($colRepeats < 1 || $colIndex + $colRepeats - 1 > AddressRange::MAX_COLUMN_INT) { + throw new Exception("Invalid number-columns-repeated $colRepeats following column $colID"); + } + } + + private static function checkColumnsRepeatedInt(int $colIndex, int $colRepeats): void + { + // We don't have column string at this point, + // and colIndex is actually 1 less than it should be. + if ($colRepeats < 1 || $colIndex + $colRepeats > AddressRange::MAX_COLUMN_INT) { + throw new Exception("Invalid number-columns-repeated $colRepeats following column index $colIndex"); + } + } + private function loadDom(string $file, ZipArchive $zip): DOMDocument { $dom = new DOMDocument('1.01', 'UTF-8'); diff --git a/tests/PhpSpreadsheetTests/Reader/Ods/BadRepeatsTest.php b/tests/PhpSpreadsheetTests/Reader/Ods/BadRepeatsTest.php new file mode 100644 index 000000000..2a8cb7758 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Ods/BadRepeatsTest.php @@ -0,0 +1,48 @@ +expectException(ReaderException::class); + $this->expectExceptionMessage('Invalid number-columns-repeated'); + $reader = new Ods(); + $infile = 'tests/data/Reader/Ods/BadRepeatCol.ods'; + $reader->load($infile); + } + + public function testBadRepeatedColsWorksheetInfo(): void + { + $this->expectException(ReaderException::class); + $this->expectExceptionMessage('Invalid number-columns-repeated'); + $reader = new Ods(); + $infile = 'tests/data/Reader/Ods/BadRepeatCol.ods'; + $reader->listWorksheetInfo($infile); + } + + public function testBadRepeatedRowsRead(): void + { + $this->expectException(ReaderException::class); + $this->expectExceptionMessage('Invalid number-rows-repeated'); + $reader = new Ods(); + $infile = 'tests/data/Reader/Ods/BadRepeatRow.ods'; + $reader->load($infile); + } + + public function testBadRepeatedRowsWorksheetInfo(): void + { + $this->expectException(ReaderException::class); + $this->expectExceptionMessage('Invalid number-rows-repeated'); + $reader = new Ods(); + $infile = 'tests/data/Reader/Ods/BadRepeatRow.ods'; + $reader->listWorksheetInfo($infile); + } +} diff --git a/tests/data/Reader/Ods/BadRepeatCol.ods b/tests/data/Reader/Ods/BadRepeatCol.ods new file mode 100644 index 000000000..2cc403756 Binary files /dev/null and b/tests/data/Reader/Ods/BadRepeatCol.ods differ diff --git a/tests/data/Reader/Ods/BadRepeatRow.ods b/tests/data/Reader/Ods/BadRepeatRow.ods new file mode 100644 index 000000000..d83a15e15 Binary files /dev/null and b/tests/data/Reader/Ods/BadRepeatRow.ods differ