diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index e9787962..8c689f11 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -4020,11 +4020,6 @@ parameters: count: 1 path: src/PhpSpreadsheet/Shared/OLERead.php - - - message: "#^Argument of an invalid type array\\\\|false supplied for foreach, only iterables are supported\\.$#" - count: 1 - path: src/PhpSpreadsheet/Shared/PasswordHasher.php - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\StringHelper\\:\\:sanitizeUTF8\\(\\) should return string but returns string\\|false\\.$#" count: 1 @@ -6635,11 +6630,6 @@ parameters: count: 1 path: tests/PhpSpreadsheetTests/Worksheet/RowCellIterator2Test.php - - - message: "#^Parameter \\#1 \\$options of static method PhpOffice\\\\PhpSpreadsheet\\\\Settings\\:\\:setLibXmlLoaderOptions\\(\\) expects int, null given\\.$#" - count: 1 - path: tests/PhpSpreadsheetTests/Worksheet/WorksheetNamedRangesTest.php - - message: "#^Property PhpOffice\\\\PhpSpreadsheetTests\\\\Writer\\\\Csv\\\\CsvEnclosureTest\\:\\:\\$cellValues has no typehint specified\\.$#" count: 1 @@ -6680,11 +6670,6 @@ parameters: count: 3 path: tests/PhpSpreadsheetTests/Writer/Html/HtmlCommentsTest.php - - - message: "#^Parameter \\#1 \\$options of static method PhpOffice\\\\PhpSpreadsheet\\\\Settings\\:\\:setLibXmlLoaderOptions\\(\\) expects int, null given\\.$#" - count: 1 - path: tests/PhpSpreadsheetTests/Writer/Xlsx/FloatsRetainedTest.php - - message: "#^Parameter \\#2 \\$locale of function setlocale expects string\\|null, string\\|false given\\.$#" count: 1 @@ -6695,16 +6680,6 @@ parameters: count: 1 path: tests/PhpSpreadsheetTests/Writer/Xlsx/LocaleFloatsTest.php - - - message: "#^Parameter \\#1 \\$options of static method PhpOffice\\\\PhpSpreadsheet\\\\Settings\\:\\:setLibXmlLoaderOptions\\(\\) expects int, null given\\.$#" - count: 2 - path: tests/PhpSpreadsheetTests/Writer/Xlsx/StartsWithHashTest.php - - - - message: "#^Parameter \\#1 \\$options of static method PhpOffice\\\\PhpSpreadsheet\\\\Settings\\:\\:setLibXmlLoaderOptions\\(\\) expects int, null given\\.$#" - count: 2 - path: tests/PhpSpreadsheetTests/Writer/Xlsx/UnparsedDataCloneTest.php - - message: "#^Cannot call method getDrawingCollection\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\Worksheet\\|null\\.$#" count: 4 @@ -6715,11 +6690,6 @@ parameters: count: 4 path: tests/PhpSpreadsheetTests/Writer/Xlsx/UnparsedDataCloneTest.php - - - message: "#^Parameter \\#1 \\$options of static method PhpOffice\\\\PhpSpreadsheet\\\\Settings\\:\\:setLibXmlLoaderOptions\\(\\) expects int, null given\\.$#" - count: 1 - path: tests/PhpSpreadsheetTests/Writer/Xlsx/UnparsedDataTest.php - - message: "#^Parameter \\#1 \\$data of function simplexml_load_string expects string, string\\|false given\\.$#" count: 2 diff --git a/src/PhpSpreadsheet/Shared/PasswordHasher.php b/src/PhpSpreadsheet/Shared/PasswordHasher.php index 9fefe88f..ba30fc31 100644 --- a/src/PhpSpreadsheet/Shared/PasswordHasher.php +++ b/src/PhpSpreadsheet/Shared/PasswordHasher.php @@ -2,11 +2,13 @@ namespace PhpOffice\PhpSpreadsheet\Shared; -use PhpOffice\PhpSpreadsheet\Exception; +use PhpOffice\PhpSpreadsheet\Exception as SpException; use PhpOffice\PhpSpreadsheet\Worksheet\Protection; class PasswordHasher { + const MAX_PASSWORD_LENGTH = 255; + /** * Get algorithm name for PHP. */ @@ -34,36 +36,40 @@ class PasswordHasher return $mapping[$algorithmName]; } - throw new Exception('Unsupported password algorithm: ' . $algorithmName); + throw new SpException('Unsupported password algorithm: ' . $algorithmName); } /** * Create a password hash from a given string. * - * This method is based on the algorithm provided by + * This method is based on the spec at: + * https://interoperability.blob.core.windows.net/files/MS-OFFCRYPTO/[MS-OFFCRYPTO].pdf + * 2.3.7.1 Binary Document Password Verifier Derivation Method 1 + * + * It replaces a method based on the algorithm provided by * Daniel Rentz of OpenOffice and the PEAR package * Spreadsheet_Excel_Writer by Xavier Noguer . * + * Scrutinizer will squawk at the use of bitwise operations here, + * but it should ultimately pass. + * * @param string $pPassword Password to hash */ private static function defaultHashPassword(string $pPassword): string { - $password = 0x0000; - $charPos = 1; // char position - - // split the plain text password in its component characters - $chars = preg_split('//', $pPassword, -1, PREG_SPLIT_NO_EMPTY); - foreach ($chars as $char) { - $value = ord($char) << $charPos++; // shifted ASCII value - $rotated_bits = $value >> 15; // rotated bits beyond bit 15 - $value &= 0x7fff; // first 15 bits - $password ^= ($value | $rotated_bits); + $verifier = 0; + $pwlen = strlen($pPassword); + $passwordArray = pack('c', $pwlen) . $pPassword; + for ($i = $pwlen; $i >= 0; --$i) { + $intermediate1 = (($verifier & 0x4000) === 0) ? 0 : 1; + $intermediate2 = 2 * $verifier; + $intermediate2 = $intermediate2 & 0x7fff; + $intermediate3 = $intermediate1 | $intermediate2; + $verifier = $intermediate3 ^ ord($passwordArray[$i]); } + $verifier ^= 0xCE4B; - $password ^= strlen($pPassword); - $password ^= 0xCE4B; - - return strtoupper(dechex($password)); + return strtoupper(dechex($verifier)); } /** @@ -82,6 +88,9 @@ class PasswordHasher */ public static function hashPassword(string $password, string $algorithm = '', string $salt = '', int $spinCount = 10000): string { + if (strlen($password) > self::MAX_PASSWORD_LENGTH) { + throw new SpException('Password exceeds ' . self::MAX_PASSWORD_LENGTH . ' characters'); + } $phpAlgorithm = self::getAlgorithm($algorithm); if (!$phpAlgorithm) { return self::defaultHashPassword($password); diff --git a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php index 6b5fa6fd..e8377ee9 100644 --- a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php +++ b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php @@ -2138,7 +2138,7 @@ class Worksheet extends BIFFwriter private function writePassword(): void { // Exit unless sheet protection and password have been specified - if (!$this->phpSheet->getProtection()->getSheet() || !$this->phpSheet->getProtection()->getPassword()) { + if (!$this->phpSheet->getProtection()->getSheet() || !$this->phpSheet->getProtection()->getPassword() || $this->phpSheet->getProtection()->getAlgorithm() !== '') { return; } diff --git a/tests/PhpSpreadsheetTests/SettingsTest.php b/tests/PhpSpreadsheetTests/SettingsTest.php index 11b93ae6..80dc07ab 100644 --- a/tests/PhpSpreadsheetTests/SettingsTest.php +++ b/tests/PhpSpreadsheetTests/SettingsTest.php @@ -41,6 +41,7 @@ class SettingsTest extends TestCase public function testSetXMLSettings(): void { + $original = Settings::getLibXmlLoaderOptions(); Settings::setLibXmlLoaderOptions(LIBXML_DTDLOAD | LIBXML_DTDATTR | LIBXML_DTDVALID); $result = Settings::getLibXmlLoaderOptions(); self::assertTrue((bool) ((LIBXML_DTDLOAD | LIBXML_DTDATTR | LIBXML_DTDVALID) & $result)); @@ -48,5 +49,6 @@ class SettingsTest extends TestCase if (\PHP_VERSION_ID < 80000) { self::assertFalse(libxml_disable_entity_loader()); } + Settings::setLibXmlLoaderOptions($original); } } diff --git a/tests/PhpSpreadsheetTests/Shared/PasswordHasherTest.php b/tests/PhpSpreadsheetTests/Shared/PasswordHasherTest.php index e85b9fa3..4b7923d8 100644 --- a/tests/PhpSpreadsheetTests/Shared/PasswordHasherTest.php +++ b/tests/PhpSpreadsheetTests/Shared/PasswordHasherTest.php @@ -2,6 +2,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Shared; +use PhpOffice\PhpSpreadsheet\Exception as SpException; use PhpOffice\PhpSpreadsheet\Shared\PasswordHasher; use PHPUnit\Framework\TestCase; @@ -14,6 +15,9 @@ class PasswordHasherTest extends TestCase */ public function testHashPassword($expectedResult, ...$args): void { + if ($expectedResult === 'exception') { + $this->expectException(SpException::class); + } $result = PasswordHasher::hashPassword(...$args); self::assertEquals($expectedResult, $result); } diff --git a/tests/PhpSpreadsheetTests/Shared/PasswordReloadTest.php b/tests/PhpSpreadsheetTests/Shared/PasswordReloadTest.php new file mode 100644 index 00000000..b6c28b17 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Shared/PasswordReloadTest.php @@ -0,0 +1,51 @@ +getActiveSheet(); + $sheet->getCell('A1')->setValue(1); + $protection = $sheet->getProtection(); + $protection->setAlgorithm($algorithm); + $protection->setPassword($password); + $protection->setSheet(true); + + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, $format); + $resheet = $reloadedSpreadsheet->getActiveSheet(); + $reprot = $resheet->getProtection(); + $repassword = $reprot->getPassword(); + $hash = ''; + if ($supported) { + $readAlgorithm = $reprot->getAlgorithm(); + self::assertSame($algorithm, $readAlgorithm); + $salt = $reprot->getSalt(); + $spin = $reprot->getSpinCount(); + $hash = PasswordHasher::hashPassword($password, $readAlgorithm, $salt, $spin); + } + self::assertSame($repassword, $hash); + $spreadsheet->disconnectWorksheets(); + $reloadedSpreadsheet->disconnectWorksheets(); + } + + public function providerPasswords(): array + { + return [ + 'Xls basic algorithm' => ['Xls', ''], + 'Xls cannot use SHA512' => ['Xls', 'SHA-512', false], + 'Xlsx basic algorithm' => ['Xlsx', ''], + 'Xlsx can use SHA512' => ['Xlsx', 'SHA-512'], + ]; + } +} diff --git a/tests/PhpSpreadsheetTests/Worksheet/WorksheetNamedRangesTest.php b/tests/PhpSpreadsheetTests/Worksheet/WorksheetNamedRangesTest.php index 1560f1ed..b367583b 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/WorksheetNamedRangesTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/WorksheetNamedRangesTest.php @@ -4,7 +4,6 @@ namespace PhpOffice\PhpSpreadsheetTests\Worksheet; use PhpOffice\PhpSpreadsheet\Exception; use PhpOffice\PhpSpreadsheet\Reader\Xlsx; -use PhpOffice\PhpSpreadsheet\Settings; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PHPUnit\Framework\TestCase; @@ -17,8 +16,6 @@ class WorksheetNamedRangesTest extends TestCase protected function setUp(): void { - Settings::setLibXmlLoaderOptions(null); // reset to default options - $reader = new Xlsx(); $this->spreadsheet = $reader->load('tests/data/Worksheet/namedRangeTest.xlsx'); } diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/ConditionalTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/ConditionalTest.php index 21df6ed6..21659dad 100644 --- a/tests/PhpSpreadsheetTests/Writer/Xlsx/ConditionalTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/ConditionalTest.php @@ -2,7 +2,6 @@ namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx; -use PhpOffice\PhpSpreadsheet\Settings; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Style\Conditional; use PhpOffice\PhpSpreadsheet\Style\Fill; @@ -11,24 +10,6 @@ use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional; class ConditionalTest extends AbstractFunctional { - /** - * @var int - */ - private $prevValue; - - protected function setUp(): void - { - $this->prevValue = Settings::getLibXmlLoaderOptions(); - - // Disable validating XML with the DTD - Settings::setLibXmlLoaderOptions($this->prevValue & ~LIBXML_DTDVALID & ~LIBXML_DTDATTR & ~LIBXML_DTDLOAD); - } - - protected function tearDown(): void - { - Settings::setLibXmlLoaderOptions($this->prevValue); - } - /** * Test check if conditional style with type 'notContainsText' works on xlsx. */ diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/DrawingsTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/DrawingsTest.php index 88c63306..012cdbcd 100644 --- a/tests/PhpSpreadsheetTests/Writer/Xlsx/DrawingsTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/DrawingsTest.php @@ -4,30 +4,11 @@ namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx; use PhpOffice\PhpSpreadsheet\IOFactory; use PhpOffice\PhpSpreadsheet\Reader\Xlsx; -use PhpOffice\PhpSpreadsheet\Settings; use PhpOffice\PhpSpreadsheet\Shared\File; use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional; class DrawingsTest extends AbstractFunctional { - /** - * @var int - */ - private $prevValue; - - protected function setUp(): void - { - $this->prevValue = Settings::getLibXmlLoaderOptions(); - - // Disable validating XML with the DTD - Settings::setLibXmlLoaderOptions($this->prevValue & ~LIBXML_DTDVALID & ~LIBXML_DTDATTR & ~LIBXML_DTDLOAD); - } - - protected function tearDown(): void - { - Settings::setLibXmlLoaderOptions($this->prevValue); - } - /** * Test save and load XLSX file with drawing on 2nd worksheet. */ diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/FloatsRetainedTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/FloatsRetainedTest.php index 22f3284b..6ba8316b 100644 --- a/tests/PhpSpreadsheetTests/Writer/Xlsx/FloatsRetainedTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/FloatsRetainedTest.php @@ -3,7 +3,6 @@ namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx; use PhpOffice\PhpSpreadsheet\Reader\Xlsx as Reader; -use PhpOffice\PhpSpreadsheet\Settings; use PhpOffice\PhpSpreadsheet\Shared\File; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Writer\Xlsx as Writer; @@ -19,7 +18,6 @@ class FloatsRetainedTest extends TestCase public function testIntyFloatsRetainedByWriter($value): void { $outputFilename = File::temporaryFilename(); - Settings::setLibXmlLoaderOptions(null); $sheet = new Spreadsheet(); $sheet->getActiveSheet()->getCell('A1')->setValue($value); diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/StartsWithHashTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/StartsWithHashTest.php index 826c482d..ebc3c92f 100644 --- a/tests/PhpSpreadsheetTests/Writer/Xlsx/StartsWithHashTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/StartsWithHashTest.php @@ -4,7 +4,6 @@ namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx; use PhpOffice\PhpSpreadsheet\Cell\DataType; use PhpOffice\PhpSpreadsheet\Reader\Xlsx as Reader; -use PhpOffice\PhpSpreadsheet\Settings; use PhpOffice\PhpSpreadsheet\Shared\File; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Writer\Xlsx as Writer; @@ -16,7 +15,6 @@ class StartsWithHashTest extends TestCase public function testStartWithHash(): void { $outputFilename = File::temporaryFilename(); - Settings::setLibXmlLoaderOptions(null); $spreadsheet = new Spreadsheet(); $sheet = $spreadsheet->getActiveSheet(); $sheet->setCellValueExplicit('A1', '#define M', DataType::TYPE_STRING); @@ -41,7 +39,6 @@ class StartsWithHashTest extends TestCase { // Make sure raw data indicates A3 is an error, but A2 isn't. $outputFilename = File::temporaryFilename(); - Settings::setLibXmlLoaderOptions(null); $spreadsheet = new Spreadsheet(); $sheet = $spreadsheet->getActiveSheet(); $sheet->setCellValueExplicit('A1', '#define M', DataType::TYPE_STRING); diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/UnparsedDataCloneTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/UnparsedDataCloneTest.php index def6f70e..9a801015 100644 --- a/tests/PhpSpreadsheetTests/Writer/Xlsx/UnparsedDataCloneTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/UnparsedDataCloneTest.php @@ -2,7 +2,6 @@ namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx; -use PhpOffice\PhpSpreadsheet\Settings; use PhpOffice\PhpSpreadsheet\Shared\File; use PHPUnit\Framework\TestCase; use ZipArchive; @@ -16,7 +15,6 @@ class UnparsedDataCloneTest extends TestCase { $sampleFilename = 'tests/data/Writer/XLSX/drawing_on_2nd_page.xlsx'; $resultFilename = File::temporaryFilename(); - Settings::setLibXmlLoaderOptions(null); // reset to default options $reader = new \PhpOffice\PhpSpreadsheet\Reader\Xlsx(); $spreadsheet = $reader->load($sampleFilename); $spreadsheet->setActiveSheetIndex(1); @@ -63,7 +61,6 @@ class UnparsedDataCloneTest extends TestCase $resultFilename1 = File::temporaryFilename(); $resultFilename2 = File::temporaryFilename(); self::assertNotEquals($resultFilename1, $resultFilename2); - Settings::setLibXmlLoaderOptions(null); // reset to default options $reader = new \PhpOffice\PhpSpreadsheet\Reader\Xlsx(); $spreadsheet = $reader->load($sampleFilename); $sheet = $spreadsheet->setActiveSheetIndex(1); diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/UnparsedDataTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/UnparsedDataTest.php index a2f21aef..56193419 100644 --- a/tests/PhpSpreadsheetTests/Writer/Xlsx/UnparsedDataTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/UnparsedDataTest.php @@ -17,7 +17,6 @@ class UnparsedDataTest extends TestCase { $sampleFilename = 'tests/data/Writer/XLSX/form_pass_print.xlsm'; $resultFilename = File::temporaryFilename(); - Settings::setLibXmlLoaderOptions(null); // reset to default options $reader = new \PhpOffice\PhpSpreadsheet\Reader\Xlsx(); $excel = $reader->load($sampleFilename); diff --git a/tests/data/Shared/PasswordHashes.php b/tests/data/Shared/PasswordHashes.php index 34c25cef..c46adf14 100644 --- a/tests/data/Shared/PasswordHashes.php +++ b/tests/data/Shared/PasswordHashes.php @@ -51,4 +51,9 @@ return [ 'Symbols_salt', 100000, ], + // Additional tests suggested by Issue #1897 + ['DCDF', 'ABCDEFGHIJKLMNOPQRSTUVW'], + ['ECD1', 'ABCDEFGHIJKLMNOPQRSTUVWX'], + ['88D2', 'ABCDEFGHIJKLMNOPQRSTUVWXY'], + 'password too long' => ['exception', str_repeat('x', 256)], ];