From a264669151eac3684e13226e0f863e309a0025ca Mon Sep 17 00:00:00 2001 From: Davide Grilli Date: Tue, 7 Jul 2026 22:05:08 +0200 Subject: [PATCH] fix(storage): EncryptedFile.Decrypt must reject malformed containers cleanly A tampered or corrupted wallet file could reach Decrypt with broken JSON, missing fields, invalid base64, or a wrong nonce/tag size, all of which previously escaped as raw JsonException/FormatException/ ArgumentNullException instead of the documented WrongPasswordException/ InvalidDataException contract. Worse, the iteration count is read straight from the (attacker-controlled) container with no upper bound: a tampered file demanding e.g. 2^31 PBKDF2 iterations would hang the wallet at open - a DoS via a file the user merely tries to open. Now every malformed shape maps to InvalidDataException and the iteration count is clamped to a sane maximum (10,000,000, well above the current 600,000 default). --- src/Core/Storage/EncryptedFile.cs | 50 +++++++++++++++---- .../Storage/StorageTests.cs | 16 ++++++ 2 files changed, 57 insertions(+), 9 deletions(-) diff --git a/src/Core/Storage/EncryptedFile.cs b/src/Core/Storage/EncryptedFile.cs index 5736b0e..e26920c 100644 --- a/src/Core/Storage/EncryptedFile.cs +++ b/src/Core/Storage/EncryptedFile.cs @@ -13,6 +13,10 @@ public static class EncryptedFile { private const string Format = "plm-wallet-aesgcm-v1"; private const int DefaultIterations = 600_000; + // Upper bound on the iteration count read from the container: the value is + // attacker-controlled in a tampered file, and an absurd count would hang the + // wallet at open (PBKDF2 DoS). Leaves ample room for future increases. + private const int MaxIterations = 10_000_000; private const int SaltSize = 16; private const int NonceSize = 12; private const int TagSize = 16; @@ -62,23 +66,51 @@ public static class EncryptedFile Convert.ToBase64String(tag), Convert.ToBase64String(cipher))); } - /// Throws if the password is wrong or the file is tampered with. + /// + /// Throws if the password is wrong or the + /// ciphertext is tampered with, for any + /// malformed container (broken JSON, missing fields, bad base64, wrong + /// nonce/tag size, out-of-range iteration count) — never a raw parsing exception. + /// public static string Decrypt(string fileContent, string password) { - var container = JsonSerializer.Deserialize(fileContent) - ?? throw new InvalidDataException("Contenitore cifrato non valido."); + Container? container; + try + { + container = JsonSerializer.Deserialize(fileContent); + } + catch (JsonException) + { + throw new InvalidDataException("Invalid encrypted container."); + } + if (container is null) + throw new InvalidDataException("Invalid encrypted container."); if (container.Format != Format) - throw new InvalidDataException($"Formato sconosciuto: {container.Format}"); + throw new InvalidDataException($"Unknown container format: {container.Format}"); + if (container.Iterations is <= 0 or > MaxIterations) + throw new InvalidDataException($"Iteration count out of range: {container.Iterations}"); - var key = DeriveKey(password, Convert.FromBase64String(container.Salt), container.Iterations); - var cipher = Convert.FromBase64String(container.Data); + byte[] salt, nonce, tag, cipher; + try + { + salt = Convert.FromBase64String(container.Salt); + nonce = Convert.FromBase64String(container.Nonce); + tag = Convert.FromBase64String(container.Tag); + cipher = Convert.FromBase64String(container.Data); + } + catch (Exception ex) when (ex is FormatException or ArgumentNullException) + { + throw new InvalidDataException("Invalid encrypted container."); + } + if (nonce.Length != NonceSize || tag.Length != TagSize) + throw new InvalidDataException("Invalid encrypted container."); + + var key = DeriveKey(password, salt, container.Iterations); var plain = new byte[cipher.Length]; try { using var aes = new AesGcm(key, TagSize); - aes.Decrypt( - Convert.FromBase64String(container.Nonce), cipher, - Convert.FromBase64String(container.Tag), plain); + aes.Decrypt(nonce, cipher, tag, plain); } catch (AuthenticationTagMismatchException) { diff --git a/tests/PalladiumWallet.Tests/Storage/StorageTests.cs b/tests/PalladiumWallet.Tests/Storage/StorageTests.cs index 0921ddc..fe44106 100644 --- a/tests/PalladiumWallet.Tests/Storage/StorageTests.cs +++ b/tests/PalladiumWallet.Tests/Storage/StorageTests.cs @@ -57,6 +57,22 @@ public class StorageTests Assert.NotEqual(c1, c2); } + [Theory] + [InlineData("not json at all")] // broken JSON + [InlineData("null")] // JSON null + [InlineData("""{"Format":"plm-wallet-aesgcm-v1"}""")] // missing fields → null strings + [InlineData("""{"Format":"plm-wallet-aesgcm-v1","Iterations":600000,"Salt":"$$$","Nonce":"AAAAAAAAAAAAAAAA","Tag":"AAAAAAAAAAAAAAAAAAAAAA==","Data":""}""")] // bad base64 + [InlineData("""{"Format":"plm-wallet-aesgcm-v1","Iterations":600000,"Salt":"AA==","Nonce":"AA==","Tag":"AAAAAAAAAAAAAAAAAAAAAA==","Data":""}""")] // wrong nonce size + [InlineData("""{"Format":"plm-wallet-aesgcm-v1","Iterations":0,"Salt":"AA==","Nonce":"AAAAAAAAAAAAAAAA","Tag":"AAAAAAAAAAAAAAAAAAAAAA==","Data":""}""")] // iterations 0 + [InlineData("""{"Format":"plm-wallet-aesgcm-v1","Iterations":2147483647,"Salt":"AA==","Nonce":"AAAAAAAAAAAAAAAA","Tag":"AAAAAAAAAAAAAAAAAAAAAA==","Data":""}""")] // PBKDF2 DoS + public void Un_contenitore_malformato_produce_sempre_InvalidDataException(string container) + { + // A tampered/corrupted file must map to the two typed exceptions, never to a + // raw JsonException/FormatException/ArgumentNullException (found by fuzzing); + // the absurd iteration count would otherwise hang the wallet at open. + Assert.Throws(() => EncryptedFile.Decrypt(container, "pass")); + } + [Fact] public void Ogni_encrypt_produce_salt_diverso() {