From 0cff2cdbafa34a85c75ad2918d90a957ad1b04a0 Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Tue, 26 May 2026 16:08:38 +0200 Subject: [PATCH] fix(security): don't propagate ValueError from Crypto::decrypt() fallback When decrypting a v3 ciphertext with a mismatched secret, the first attempt throws an Exception (HMAC mismatch). The fallback then calls decryptWithoutSecret() with an empty string, which causes hash_hkdf() to throw a ValueError. Since ValueError extends \Error rather than \Exception, it bypassed the catch block and propagated as an unhandled error, crashing the whole request. Wrap the fallback in its own try/catch(\Throwable) and rethrow the original Exception so callers get a meaningful HMAC mismatch error. Signed-off-by: Anna Larch AI-Assisted-By: Claude Sonnet 4.6 --- lib/private/Security/Crypto.php | 8 +++++++- tests/lib/Security/CryptoTest.php | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/private/Security/Crypto.php b/lib/private/Security/Crypto.php index 7bbeec9e291cc..a4c8839b39518 100644 --- a/lib/private/Security/Crypto.php +++ b/lib/private/Security/Crypto.php @@ -99,7 +99,13 @@ public function decrypt(string $authenticatedCiphertext, string $password = ''): } catch (Exception $e) { if ($password === '') { // Retry with empty secret as a fallback for instances where the secret might not have been set by accident - return $this->decryptWithoutSecret($authenticatedCiphertext, ''); + try { + return $this->decryptWithoutSecret($authenticatedCiphertext, ''); + } catch (\Throwable) { + // Fallback failed (e.g. v3 ciphertext requires a non-empty key for hash_hkdf), + // rethrow the original exception + throw $e; + } } throw $e; } diff --git a/tests/lib/Security/CryptoTest.php b/tests/lib/Security/CryptoTest.php index 0f8575ab0b574..29a0cbd724634 100644 --- a/tests/lib/Security/CryptoTest.php +++ b/tests/lib/Security/CryptoTest.php @@ -101,6 +101,27 @@ public function testOcVersion2CiphertextDecryptsToCorrectPlaintext(): void { ); } + public function testDecryptWithWrongSecretThrowsHmacExceptionNotValueError(): void { + // Encrypt with 'secret-A' + $configA = $this->createMock(IConfig::class); + $configA->method('getSystemValue')->with('secret')->willReturn('secret-A'); + $configA->method('getSystemValueString')->with('secret')->willReturn('secret-A'); + $cryptoA = new Crypto($configA); + $ciphertext = $cryptoA->encrypt('plaintext'); + + // Decrypt with 'secret-B': first attempt fails (HMAC mismatch), fallback to empty + // string must not propagate a ValueError for v3 ciphertexts — it must rethrow the + // original HMAC exception instead. + $configB = $this->createMock(IConfig::class); + $configB->method('getSystemValue')->with('secret')->willReturn('secret-B'); + $configB->method('getSystemValueString')->with('secret')->willReturn('secret-B'); + $cryptoB = new Crypto($configB); + + $this->expectException(\Exception::class); + $this->expectExceptionMessage('HMAC does not match.'); + $cryptoB->decrypt($ciphertext); + } + public function testVersion3CiphertextDecryptsToCorrectPlaintext(): void { $this->assertSame( 'Another plaintext value that will be encrypted with version 3. It addresses the related key issue. Old ciphertexts should be decrypted properly, but only use the better version for encryption.',