From 08a6e7549f7bc5730cd0b030d7992823ac7cea3a Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Fri, 3 Oct 2025 13:07:36 -0700 Subject: [PATCH] More Sophisticated Workbook Password Algorithms (Xlsx only) Fix #4673. Our password hasher can handle different algorithms, but the workbook password (for maintaining the structure of the workbook, not for encrypting the entire workbook) currently supports only the single algorithm that was in place many years ago. Expand it, and Xlsx Reader and Writer, to be able to use, say, SHA-512, which is what Excel itself uses. The revisions password needs to be expanded in the same way as the workbook password. It is used for file sharing, but MS has deprecated it because it feels that modern technologies introduce better ways to accomplish what it was needed for. You'll need to look pretty hard to even find it in Excel - it's not, for example, on any of the ribbons. Nevertheless, the solution here is pretty much identical to the solution for the workbook password, so I am fixing it at the same time. --- .php-cs-fixer.dist.php | 2 +- src/PhpSpreadsheet/Document/Security.php | 141 ++++++++++++++++-- src/PhpSpreadsheet/Reader/Xlsx.php | 66 +++++++- src/PhpSpreadsheet/Shared/PasswordHasher.php | 2 +- src/PhpSpreadsheet/Writer/Xlsx/Workbook.php | 35 +++-- .../Document/SecurityTest.php | 83 ++++++++++- 6 files changed, 300 insertions(+), 29 deletions(-) diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 562e7fe81b..4050b03617 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -84,6 +84,7 @@ 'method_chaining_indentation' => true, 'modernize_strpos' => true, 'modernize_types_casting' => true, + 'modifier_keywords' => ['elements' => ['property', 'method']], // not const 'multiline_comment_opening_closing' => true, 'multiline_whitespace_before_semicolons' => true, 'native_constant_invocation' => false, // Micro optimization that look messy @@ -236,7 +237,6 @@ 'types_spaces' => true, 'unary_operator_spaces' => true, 'use_arrow_functions' => true, - 'visibility_required' => ['elements' => ['property', 'method']], // not const 'void_return' => true, 'whitespace_after_comma_in_array' => true, 'yoda_style' => false, diff --git a/src/PhpSpreadsheet/Document/Security.php b/src/PhpSpreadsheet/Document/Security.php index 31a32beb42..c65072a722 100644 --- a/src/PhpSpreadsheet/Document/Security.php +++ b/src/PhpSpreadsheet/Document/Security.php @@ -31,12 +31,21 @@ class Security */ private string $workbookPassword = ''; - /** - * Create a new Document Security instance. - */ - public function __construct() - { - } + private string $workbookAlgorithmName = ''; + + private string $workbookHashValue = ''; + + private string $workbookSaltValue = ''; + + private int $workbookSpinCount = 0; + + private string $revisionsAlgorithmName = ''; + + private string $revisionsHashValue = ''; + + private string $revisionsSaltValue = ''; + + private int $revisionsSpinCount = 0; /** * Is some sort of document security enabled? @@ -105,10 +114,18 @@ public function getRevisionsPassword(): string public function setRevisionsPassword(?string $password, bool $alreadyHashed = false): static { if ($password !== null) { - if (!$alreadyHashed) { - $password = PasswordHasher::hashPassword($password); + if ($this->advancedRevisionsPassword()) { + if (!$alreadyHashed) { + $password = PasswordHasher::hashPassword($password, $this->revisionsAlgorithmName, $this->revisionsSaltValue, $this->revisionsSpinCount); + } + $this->revisionsHashValue = $password; + $this->revisionsPassword = ''; + } else { + if (!$alreadyHashed) { + $password = PasswordHasher::hashPassword($password); + } + $this->revisionsPassword = $password; } - $this->revisionsPassword = $password; } return $this; @@ -129,12 +146,112 @@ public function getWorkbookPassword(): string public function setWorkbookPassword(?string $password, bool $alreadyHashed = false): static { if ($password !== null) { - if (!$alreadyHashed) { - $password = PasswordHasher::hashPassword($password); + if ($this->advancedPassword()) { + if (!$alreadyHashed) { + $password = PasswordHasher::hashPassword($password, $this->workbookAlgorithmName, $this->workbookSaltValue, $this->workbookSpinCount); + } + $this->workbookHashValue = $password; + $this->workbookPassword = ''; + } else { + if (!$alreadyHashed) { + $password = PasswordHasher::hashPassword($password); + } + $this->workbookPassword = $password; } - $this->workbookPassword = $password; } return $this; } + + public function getWorkbookHashValue(): string + { + return $this->advancedPassword() ? $this->workbookHashValue : ''; + } + + public function advancedPassword(): bool + { + return $this->workbookAlgorithmName !== '' && $this->workbookSaltValue !== '' && $this->workbookSpinCount > 0; + } + + public function getWorkbookAlgorithmName(): string + { + return $this->workbookAlgorithmName; + } + + public function setWorkbookAlgorithmName(string $workbookAlgorithmName): static + { + $this->workbookAlgorithmName = $workbookAlgorithmName; + + return $this; + } + + public function getWorkbookSpinCount(): int + { + return $this->workbookSpinCount; + } + + public function setWorkbookSpinCount(int $workbookSpinCount): static + { + $this->workbookSpinCount = $workbookSpinCount; + + return $this; + } + + public function getWorkbookSaltValue(): string + { + return $this->workbookSaltValue; + } + + public function setWorkbookSaltValue(string $workbookSaltValue, bool $base64Required): static + { + $this->workbookSaltValue = $base64Required ? base64_encode($workbookSaltValue) : $workbookSaltValue; + + return $this; + } + + public function getRevisionsHashValue(): string + { + return $this->advancedRevisionsPassword() ? $this->revisionsHashValue : ''; + } + + public function advancedRevisionsPassword(): bool + { + return $this->revisionsAlgorithmName !== '' && $this->revisionsSaltValue !== '' && $this->revisionsSpinCount > 0; + } + + public function getRevisionsAlgorithmName(): string + { + return $this->revisionsAlgorithmName; + } + + public function setRevisionsAlgorithmName(string $revisionsAlgorithmName): static + { + $this->revisionsAlgorithmName = $revisionsAlgorithmName; + + return $this; + } + + public function getRevisionsSpinCount(): int + { + return $this->revisionsSpinCount; + } + + public function setRevisionsSpinCount(int $revisionsSpinCount): static + { + $this->revisionsSpinCount = $revisionsSpinCount; + + return $this; + } + + public function getRevisionsSaltValue(): string + { + return $this->revisionsSaltValue; + } + + public function setRevisionsSaltValue(string $revisionsSaltValue, bool $base64Required): static + { + $this->revisionsSaltValue = $base64Required ? base64_encode($revisionsSaltValue) : $revisionsSaltValue; + + return $this; + } } diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php index f8a0b11324..5b0f164361 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx.php +++ b/src/PhpSpreadsheet/Reader/Xlsx.php @@ -2201,23 +2201,79 @@ private function readProtection(Spreadsheet $excel, SimpleXMLElement $xmlWorkboo return; } - $excel->getSecurity()->setLockRevision(self::getLockValue($xmlWorkbook->workbookProtection, 'lockRevision')); - $excel->getSecurity()->setLockStructure(self::getLockValue($xmlWorkbook->workbookProtection, 'lockStructure')); - $excel->getSecurity()->setLockWindows(self::getLockValue($xmlWorkbook->workbookProtection, 'lockWindows')); + $security = $excel->getSecurity(); + $security->setLockRevision( + self::getLockValue($xmlWorkbook->workbookProtection, 'lockRevision') + ); + $security->setLockStructure( + self::getLockValue($xmlWorkbook->workbookProtection, 'lockStructure') + ); + $security->setLockWindows( + self::getLockValue($xmlWorkbook->workbookProtection, 'lockWindows') + ); if ($xmlWorkbook->workbookProtection['revisionsPassword']) { - $excel->getSecurity()->setRevisionsPassword( + $security->setRevisionsPassword( (string) $xmlWorkbook->workbookProtection['revisionsPassword'], true ); } + if ($xmlWorkbook->workbookProtection['revisionsAlgorithmName']) { + $security->setRevisionsAlgorithmName( + (string) $xmlWorkbook->workbookProtection['revisionsAlgorithmName'] + ); + } + if ($xmlWorkbook->workbookProtection['revisionsSaltValue']) { + $security->setRevisionsSaltValue( + (string) $xmlWorkbook->workbookProtection['revisionsSaltValue'], + false + ); + } + if ($xmlWorkbook->workbookProtection['revisionsSpinCount']) { + $security->setRevisionsSpinCount( + (int) $xmlWorkbook->workbookProtection['revisionsSpinCount'] + ); + } + if ($xmlWorkbook->workbookProtection['revisionsHashValue']) { + if ($security->advancedRevisionsPassword()) { + $security->setRevisionsPassword( + (string) $xmlWorkbook->workbookProtection['revisionsHashValue'], + true + ); + } + } if ($xmlWorkbook->workbookProtection['workbookPassword']) { - $excel->getSecurity()->setWorkbookPassword( + $security->setWorkbookPassword( (string) $xmlWorkbook->workbookProtection['workbookPassword'], true ); } + + if ($xmlWorkbook->workbookProtection['workbookAlgorithmName']) { + $security->setWorkbookAlgorithmName( + (string) $xmlWorkbook->workbookProtection['workbookAlgorithmName'] + ); + } + if ($xmlWorkbook->workbookProtection['workbookSaltValue']) { + $security->setWorkbookSaltValue( + (string) $xmlWorkbook->workbookProtection['workbookSaltValue'], + false + ); + } + if ($xmlWorkbook->workbookProtection['workbookSpinCount']) { + $security->setWorkbookSpinCount( + (int) $xmlWorkbook->workbookProtection['workbookSpinCount'] + ); + } + if ($xmlWorkbook->workbookProtection['workbookHashValue']) { + if ($security->advancedPassword()) { + $security->setWorkbookPassword( + (string) $xmlWorkbook->workbookProtection['workbookHashValue'], + true + ); + } + } } private static function getLockValue(SimpleXMLElement $protection, string $key): ?bool diff --git a/src/PhpSpreadsheet/Shared/PasswordHasher.php b/src/PhpSpreadsheet/Shared/PasswordHasher.php index fcdbc982dc..4d1103a87b 100644 --- a/src/PhpSpreadsheet/Shared/PasswordHasher.php +++ b/src/PhpSpreadsheet/Shared/PasswordHasher.php @@ -78,7 +78,7 @@ private static function defaultHashPassword(string $password): string * * @param string $password Password to hash * @param string $algorithm Hash algorithm used to compute the password hash value - * @param string $salt Pseudorandom string + * @param string $salt Pseudorandom base64-encoded string * @param int $spinCount Number of times to iterate on a hash of a password * * @return string Hashed password diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php b/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php index 1f5dcce7d6..b9aa9b69c8 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php +++ b/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php @@ -125,18 +125,35 @@ private function writeBookViews(XMLWriter $objWriter, Spreadsheet $spreadsheet): */ private function writeWorkbookProtection(XMLWriter $objWriter, Spreadsheet $spreadsheet): void { - if ($spreadsheet->getSecurity()->isSecurityEnabled()) { + $security = $spreadsheet->getSecurity(); + if ($security->isSecurityEnabled()) { $objWriter->startElement('workbookProtection'); - $objWriter->writeAttribute('lockRevision', ($spreadsheet->getSecurity()->getLockRevision() ? 'true' : 'false')); - $objWriter->writeAttribute('lockStructure', ($spreadsheet->getSecurity()->getLockStructure() ? 'true' : 'false')); - $objWriter->writeAttribute('lockWindows', ($spreadsheet->getSecurity()->getLockWindows() ? 'true' : 'false')); - - if ($spreadsheet->getSecurity()->getRevisionsPassword() != '') { - $objWriter->writeAttribute('revisionsPassword', $spreadsheet->getSecurity()->getRevisionsPassword()); + $objWriter->writeAttribute('lockRevision', ($security->getLockRevision() ? 'true' : 'false')); + $objWriter->writeAttribute('lockStructure', ($security->getLockStructure() ? 'true' : 'false')); + $objWriter->writeAttribute('lockWindows', ($security->getLockWindows() ? 'true' : 'false')); + + if ($security->getRevisionsPassword() !== '') { + $objWriter->writeAttribute('revisionsPassword', $security->getRevisionsPassword()); + } else { + $hashValue = $security->getRevisionsHashValue(); + if ($hashValue !== '') { + $objWriter->writeAttribute('revisionsAlgorithmName', $security->getRevisionsAlgorithmName()); + $objWriter->writeAttribute('revisionsHashValue', $hashValue); + $objWriter->writeAttribute('revisionsSaltValue', $security->getRevisionsSaltValue()); + $objWriter->writeAttribute('revisionsSpinCount', (string) $security->getRevisionsSpinCount()); + } } - if ($spreadsheet->getSecurity()->getWorkbookPassword() != '') { - $objWriter->writeAttribute('workbookPassword', $spreadsheet->getSecurity()->getWorkbookPassword()); + if ($security->getWorkbookPassword() !== '') { + $objWriter->writeAttribute('workbookPassword', $security->getWorkbookPassword()); + } else { + $hashValue = $security->getWorkbookHashValue(); + if ($hashValue !== '') { + $objWriter->writeAttribute('workbookAlgorithmName', $security->getWorkbookAlgorithmName()); + $objWriter->writeAttribute('workbookHashValue', $hashValue); + $objWriter->writeAttribute('workbookSaltValue', $security->getWorkbookSaltValue()); + $objWriter->writeAttribute('workbookSpinCount', (string) $security->getWorkbookSpinCount()); + } } $objWriter->endElement(); diff --git a/tests/PhpSpreadsheetTests/Document/SecurityTest.php b/tests/PhpSpreadsheetTests/Document/SecurityTest.php index 75cf5106f9..7ae9b0b6f4 100644 --- a/tests/PhpSpreadsheetTests/Document/SecurityTest.php +++ b/tests/PhpSpreadsheetTests/Document/SecurityTest.php @@ -4,8 +4,13 @@ namespace PhpOffice\PhpSpreadsheetTests\Document; +use PhpOffice\PhpSpreadsheet\Document\Security; +use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException; +use PhpOffice\PhpSpreadsheet\Shared\PasswordHasher; use PhpOffice\PhpSpreadsheet\Spreadsheet; +use PhpOffice\PhpSpreadsheet\Worksheet\Protection; use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional; +use PHPUnit\Framework\Attributes\DataProvider; class SecurityTest extends AbstractFunctional { @@ -40,6 +45,66 @@ public function testSecurity(): void self::assertSame($hashedRevisionsPassword, $reloadedSecurity->getWorkbookPassword()); } + public function testHashRatherThanPassword(): void + { + $spreadsheet = new Spreadsheet(); + $spreadsheet->getActiveSheet()->getCell('A1')->setValue('Hello'); + $security = $spreadsheet->getSecurity(); + $password = '12345'; + $algorithm = Protection::ALGORITHM_SHA_512; + $salt = 'KX7zweex4Ay6KVZu9JU6Gw=='; + $spinCount = 100_000; + $hash = PasswordHasher::hashPassword($password, $algorithm, $salt, $spinCount); + $security->setLockStructure(true) + ->setWorkbookAlgorithmName($algorithm) + ->setWorkbookSaltValue($salt, false) + ->setWorkbookSpinCount($spinCount) + ->setWorkbookPassword($password); + self::assertSame('', $security->getWorkbookPassword()); + self::assertSame($hash, $security->getWorkbookHashValue()); + + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx'); + $spreadsheet->disconnectWorksheets(); + $reloadedSecurity = $reloadedSpreadsheet->getSecurity(); + self::assertTrue($reloadedSecurity->getLockStructure()); + self::assertSame('', $reloadedSecurity->getWorkbookPassword()); + self::assertSame($hash, $reloadedSecurity->getWorkbookHashValue()); + self::assertSame($algorithm, $reloadedSecurity->getWorkbookAlgorithmName()); + self::assertSame($salt, $reloadedSecurity->getWorkbookSaltValue()); + self::assertSame($spinCount, $reloadedSecurity->getWorkbookSpinCount()); + $reloadedSpreadsheet->disconnectWorksheets(); + } + + public function testRevisionsHashRatherThanPassword(): void + { + $spreadsheet = new Spreadsheet(); + $spreadsheet->getActiveSheet()->getCell('A1')->setValue('Hello'); + $security = $spreadsheet->getSecurity(); + $password = '54321'; + $algorithm = Protection::ALGORITHM_SHA_512; + $salt = 'ddXHG3GsaI5PnaiaVnFGkw=='; + $spinCount = 100_000; + $hash = PasswordHasher::hashPassword($password, $algorithm, $salt, $spinCount); + $security->setLockRevision(true) + ->setRevisionsAlgorithmName($algorithm) + ->setRevisionsSaltValue($salt, false) + ->setRevisionsSpinCount($spinCount) + ->setRevisionsPassword($password); + self::assertSame('', $security->getRevisionsPassword()); + self::assertSame($hash, $security->getRevisionsHashValue()); + + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx'); + $spreadsheet->disconnectWorksheets(); + $reloadedSecurity = $reloadedSpreadsheet->getSecurity(); + self::assertTrue($reloadedSecurity->getLockRevision()); + self::assertSame('', $reloadedSecurity->getRevisionsPassword()); + self::assertSame($hash, $reloadedSecurity->getRevisionsHashValue()); + self::assertSame($algorithm, $reloadedSecurity->getRevisionsAlgorithmName()); + self::assertSame($salt, $reloadedSecurity->getRevisionsSaltValue()); + self::assertSame($spinCount, $reloadedSecurity->getRevisionsSpinCount()); + $reloadedSpreadsheet->disconnectWorksheets(); + } + public static function providerLocks(): array { return [ @@ -54,7 +119,7 @@ public static function providerLocks(): array ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('providerLocks')] + #[DataProvider('providerLocks')] public function testLocks(bool $revision, bool $windows, bool $structure): void { $spreadsheet = new Spreadsheet(); @@ -72,4 +137,20 @@ public function testLocks(bool $revision, bool $windows, bool $structure): void self::assertSame($windows, $reloadedSecurity->getLockWindows()); self::assertSame($structure, $reloadedSecurity->getLockStructure()); } + + public function testBadAlgorithm(): void + { + $security = new Security(); + $password = '12345'; + $algorithm = 'SHA-513'; + $salt = 'KX7zweex4Ay6KVZu9JU6Gw=='; + $spinCount = 100_000; + + try { + $hash = PasswordHasher::hashPassword($password, $algorithm, $salt, $spinCount); + self::fail('hashPassword should have thrown exception'); + } catch (SpreadsheetException $e) { + self::assertStringContainsString('Unsupported password algorithm', $e->getMessage()); + } + } }