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 | 33 +++- .../Document/SecurityTest.php | 83 ++++++++++- 6 files changed, 299 insertions(+), 28 deletions(-) diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 562e7fe81..4050b0361 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -84,6 +84,7 @@ $config '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 @@ $config '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 31a32beb4..c65072a72 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 @@ class Security 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 @@ class Security 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 f8a0b1132..5b0f16436 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx.php +++ b/src/PhpSpreadsheet/Reader/Xlsx.php @@ -2201,23 +2201,79 @@ class Xlsx extends BaseReader 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 fcdbc982d..4d1103a87 100644 --- a/src/PhpSpreadsheet/Shared/PasswordHasher.php +++ b/src/PhpSpreadsheet/Shared/PasswordHasher.php @@ -78,7 +78,7 @@ class PasswordHasher * * @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 1f5dcce7d..b9aa9b69c 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php +++ b/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php @@ -125,18 +125,35 @@ class Workbook extends WriterPart */ 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')); + $objWriter->writeAttribute('lockRevision', ($security->getLockRevision() ? 'true' : 'false')); + $objWriter->writeAttribute('lockStructure', ($security->getLockStructure() ? 'true' : 'false')); + $objWriter->writeAttribute('lockWindows', ($security->getLockWindows() ? 'true' : 'false')); - if ($spreadsheet->getSecurity()->getRevisionsPassword() != '') { - $objWriter->writeAttribute('revisionsPassword', $spreadsheet->getSecurity()->getRevisionsPassword()); + 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 75cf5106f..7ae9b0b6f 100644 --- a/tests/PhpSpreadsheetTests/Document/SecurityTest.php +++ b/tests/PhpSpreadsheetTests/Document/SecurityTest.php @@ -4,8 +4,13 @@ declare(strict_types=1); 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 @@ class SecurityTest extends AbstractFunctional 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 @@ class SecurityTest extends AbstractFunctional ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('providerLocks')] + #[DataProvider('providerLocks')] public function testLocks(bool $revision, bool $windows, bool $structure): void { $spreadsheet = new Spreadsheet(); @@ -72,4 +137,20 @@ class SecurityTest extends AbstractFunctional 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()); + } + } }