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).
This commit is contained in:
@@ -13,6 +13,10 @@ public static class EncryptedFile
|
|||||||
{
|
{
|
||||||
private const string Format = "plm-wallet-aesgcm-v1";
|
private const string Format = "plm-wallet-aesgcm-v1";
|
||||||
private const int DefaultIterations = 600_000;
|
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 SaltSize = 16;
|
||||||
private const int NonceSize = 12;
|
private const int NonceSize = 12;
|
||||||
private const int TagSize = 16;
|
private const int TagSize = 16;
|
||||||
@@ -62,23 +66,51 @@ public static class EncryptedFile
|
|||||||
Convert.ToBase64String(tag), Convert.ToBase64String(cipher)));
|
Convert.ToBase64String(tag), Convert.ToBase64String(cipher)));
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>Throws <see cref="WrongPasswordException"/> if the password is wrong or the file is tampered with.</summary>
|
/// <summary>
|
||||||
|
/// Throws <see cref="WrongPasswordException"/> if the password is wrong or the
|
||||||
|
/// ciphertext is tampered with, <see cref="InvalidDataException"/> for any
|
||||||
|
/// malformed container (broken JSON, missing fields, bad base64, wrong
|
||||||
|
/// nonce/tag size, out-of-range iteration count) — never a raw parsing exception.
|
||||||
|
/// </summary>
|
||||||
public static string Decrypt(string fileContent, string password)
|
public static string Decrypt(string fileContent, string password)
|
||||||
{
|
{
|
||||||
var container = JsonSerializer.Deserialize<Container>(fileContent)
|
Container? container;
|
||||||
?? throw new InvalidDataException("Contenitore cifrato non valido.");
|
try
|
||||||
|
{
|
||||||
|
container = JsonSerializer.Deserialize<Container>(fileContent);
|
||||||
|
}
|
||||||
|
catch (JsonException)
|
||||||
|
{
|
||||||
|
throw new InvalidDataException("Invalid encrypted container.");
|
||||||
|
}
|
||||||
|
if (container is null)
|
||||||
|
throw new InvalidDataException("Invalid encrypted container.");
|
||||||
if (container.Format != Format)
|
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);
|
byte[] salt, nonce, tag, cipher;
|
||||||
var cipher = Convert.FromBase64String(container.Data);
|
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];
|
var plain = new byte[cipher.Length];
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
using var aes = new AesGcm(key, TagSize);
|
using var aes = new AesGcm(key, TagSize);
|
||||||
aes.Decrypt(
|
aes.Decrypt(nonce, cipher, tag, plain);
|
||||||
Convert.FromBase64String(container.Nonce), cipher,
|
|
||||||
Convert.FromBase64String(container.Tag), plain);
|
|
||||||
}
|
}
|
||||||
catch (AuthenticationTagMismatchException)
|
catch (AuthenticationTagMismatchException)
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -57,6 +57,22 @@ public class StorageTests
|
|||||||
Assert.NotEqual(c1, c2);
|
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<InvalidDataException>(() => EncryptedFile.Decrypt(container, "pass"));
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public void Ogni_encrypt_produce_salt_diverso()
|
public void Ogni_encrypt_produce_salt_diverso()
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user