From c868c17505c6b016e1c9e808b8f9e2f5ec8cd651 Mon Sep 17 00:00:00 2001 From: Davide Grilli Date: Tue, 7 Jul 2026 21:51:22 +0200 Subject: [PATCH] fix(crypto): Bip39.TryParse must not throw on unrecognisable text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wordlist.AutoDetect lands on NBitcoin's internal "Unknown" language for text resembling no wordlist and throws NotSupportedException instead of failing gracefully — found by fuzzing the restore-mnemonic input. TryParse now treats that (and the same failure mode from the Mnemonic constructor) as "not a valid mnemonic", consistent with every other rejection path. --- src/Core/Crypto/Bip39.cs | 45 +++++++++++++------ .../Crypto/Bip39Tests.cs | 11 +++++ 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/Core/Crypto/Bip39.cs b/src/Core/Crypto/Bip39.cs index 99ea0ed..a18c0c7 100644 --- a/src/Core/Crypto/Bip39.cs +++ b/src/Core/Crypto/Bip39.cs @@ -58,28 +58,45 @@ public static class Bip39 // ideographic spaces — the text must not be altered further (§4.1). text = text.Trim(); - IEnumerable candidates = language is not null - ? [ToWordlist(language.Value)] - : [Wordlist.AutoDetect(text)]; - - foreach (var wordlist in candidates) + Wordlist wordlist; + if (language is not null) + { + wordlist = ToWordlist(language.Value); + } + else { try { - var parsed = new Mnemonic(text, wordlist); - // The constructor does NOT verify the checksum — explicit check is mandatory. - if (parsed.IsValidChecksum && parsed.Words.Length is 12 or 15 or 18 or 21 or 24) - { - mnemonic = parsed; - return true; - } + wordlist = Wordlist.AutoDetect(text); } - catch (FormatException) + catch (NotSupportedException) { - // Words outside the wordlist or invalid count — try the next candidate. + // AutoDetect lands on NBitcoin's internal "Unknown" language for + // text resembling no wordlist and throws instead of returning a + // default (found by fuzzing) — not a valid mnemonic, plain and simple. + return false; } } + try + { + var parsed = new Mnemonic(text, wordlist); + // The constructor does NOT verify the checksum — explicit check is mandatory. + if (parsed.IsValidChecksum && parsed.Words.Length is 12 or 15 or 18 or 21 or 24) + { + mnemonic = parsed; + return true; + } + } + catch (FormatException) + { + // Words outside the wordlist or invalid count. + } + catch (NotSupportedException) + { + // Defensive: same NBitcoin failure mode surfacing from the constructor. + } + return false; } diff --git a/tests/PalladiumWallet.Tests/Crypto/Bip39Tests.cs b/tests/PalladiumWallet.Tests/Crypto/Bip39Tests.cs index 0b3c235..83c5475 100644 --- a/tests/PalladiumWallet.Tests/Crypto/Bip39Tests.cs +++ b/tests/PalladiumWallet.Tests/Crypto/Bip39Tests.cs @@ -126,4 +126,15 @@ public class Bip39Tests Assert.Throws( () => Bip39.Generate(MnemonicLength.Twelve, (MnemonicLanguage)99)); } + + [Fact] + public void Testo_che_non_somiglia_a_nessuna_wordlist_viene_rifiutato_senza_eccezioni() + { + // NBitcoin's Wordlist.AutoDetect throws NotSupportedException("Unknown") + // for text resembling no wordlist instead of failing gracefully (found by + // fuzzing): TryParse must swallow it and return false — this string reaches + // TryParse straight from the user's restore input. + Assert.False(Bip39.TryParse("ábÊco ábaco0ábaco ábÊ", out var mnemonic)); + Assert.Null(mnemonic); + } }