Skip to content

Commit 29f43d8

Browse files
committed
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 <anna@nextcloud.com> AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 0a4d4bc commit 29f43d8

2 files changed

Lines changed: 28 additions & 1 deletion

File tree

lib/private/Security/Crypto.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,13 @@ public function decrypt(string $authenticatedCiphertext, string $password = ''):
102102
} catch (Exception $e) {
103103
if ($password === '') {
104104
// Retry with empty secret as a fallback for instances where the secret might not have been set by accident
105-
return $this->decryptWithoutSecret($authenticatedCiphertext, '');
105+
try {
106+
return $this->decryptWithoutSecret($authenticatedCiphertext, '');
107+
} catch (\Throwable) {
108+
// Fallback failed (e.g. v3 ciphertext requires a non-empty key for hash_hkdf),
109+
// rethrow the original exception
110+
throw $e;
111+
}
106112
}
107113
throw $e;
108114
}

tests/lib/Security/CryptoTest.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,27 @@ public function testOcVersion2CiphertextDecryptsToCorrectPlaintext(): void {
102102
);
103103
}
104104

105+
public function testDecryptWithWrongSecretThrowsHmacExceptionNotValueError(): void {
106+
// Encrypt with 'secret-A'
107+
$configA = $this->createMock(IConfig::class);
108+
$configA->method('getSystemValue')->with('secret')->willReturn('secret-A');
109+
$configA->method('getSystemValueString')->with('secret')->willReturn('secret-A');
110+
$cryptoA = new Crypto($configA);
111+
$ciphertext = $cryptoA->encrypt('plaintext');
112+
113+
// Decrypt with 'secret-B': first attempt fails (HMAC mismatch), fallback to empty
114+
// string must not propagate a ValueError for v3 ciphertexts — it must rethrow the
115+
// original HMAC exception instead.
116+
$configB = $this->createMock(IConfig::class);
117+
$configB->method('getSystemValue')->with('secret')->willReturn('secret-B');
118+
$configB->method('getSystemValueString')->with('secret')->willReturn('secret-B');
119+
$cryptoB = new Crypto($configB);
120+
121+
$this->expectException(\Exception::class);
122+
$this->expectExceptionMessage('HMAC does not match.');
123+
$cryptoB->decrypt($ciphertext);
124+
}
125+
105126
public function testVersion3CiphertextDecryptsToCorrectPlaintext(): void {
106127
$this->assertSame(
107128
'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.',

0 commit comments

Comments
 (0)