diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 3538d091..625c3429 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -4310,11 +4310,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 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/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..6d127496 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Shared/PasswordReloadTest.php @@ -0,0 +1,63 @@ +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/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)], ];