From 7fb9a6b46d10caf3b9d5251eec82b9d189d67b3f Mon Sep 17 00:00:00 2001 From: liquizard Date: Thu, 30 Jun 2022 17:18:58 +0100 Subject: [PATCH 1/4] Fatal error for bad value passed to PublicKey constructor --- src/Solnet.Wallet/PublicKey.cs | 19 +++++++++++++++-- src/Solnet.Wallet/Utilities/Base58Encoder.cs | 22 +++++++++++++++++++- test/Solnet.Wallet.Test/KeysTest.cs | 18 ++++++++++++++++ 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/Solnet.Wallet/PublicKey.cs b/src/Solnet.Wallet/PublicKey.cs index 5107021f..0802890e 100644 --- a/src/Solnet.Wallet/PublicKey.cs +++ b/src/Solnet.Wallet/PublicKey.cs @@ -79,7 +79,9 @@ public PublicKey(byte[] key) /// The public key as base58 encoded string. public PublicKey(string key) { - Key = key ?? throw new ArgumentNullException(nameof(key)); + if (key == null) throw new ArgumentNullException(nameof(key)); + if (!FastCheck(key)) throw new ArgumentException("publickey contains a non-base58 character"); + Key = key; } /// @@ -201,10 +203,11 @@ public bool IsValid() /// Returns true if the input is a valid key, false otherwise. public static bool IsValid(string key, bool validateCurve = false) { - if(!string.IsNullOrEmpty(key)) + if (!string.IsNullOrEmpty(key)) { try { + if (!FastCheck(key)) return false; return IsValid(Encoders.Base58.DecodeData(key), validateCurve); } catch(Exception) @@ -249,6 +252,18 @@ public static bool IsValid(ReadOnlySpan key, bool validateCurve = false) return key != null && key.Length == PublicKeyLength && (!validateCurve || key.IsOnCurve()); } + /// + /// Fast validation to determine whether this is a valid public key input pattern. + /// Checks are valid characters for base58 and no whitespace. + /// Avoids performing the conversion to a buffer and checking it is actually 32 bytes as a permformance trade-off. + /// + /// public key value to check + /// true means good, false means bad + private static bool FastCheck(string value) + { + return Base58Encoder.IsValidWithoutWhitespace(value); + } + #region KeyDerivation /// diff --git a/src/Solnet.Wallet/Utilities/Base58Encoder.cs b/src/Solnet.Wallet/Utilities/Base58Encoder.cs index 09ddbac4..46939ca5 100644 --- a/src/Solnet.Wallet/Utilities/Base58Encoder.cs +++ b/src/Solnet.Wallet/Utilities/Base58Encoder.cs @@ -2,6 +2,7 @@ using System; using System.Collections; +using System.Collections.Generic; using System.Linq; namespace Solnet.Wallet.Utilities @@ -15,6 +16,7 @@ public sealed class Base58Encoder : DataEncoder /// The base58 characters. /// private static readonly char[] PszBase58 = "123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz".ToCharArray(); + private static readonly Dictionary Validator = PszBase58.ToDictionary(x => x, x => true); /// /// @@ -170,5 +172,23 @@ public override byte[] DecodeData(string encoded) vch[i2++] = b256[it2++]; return vch; } + + + /// + /// Strict validation with no whitespace allowed + /// + /// Base58 string data + /// + /// + public static bool IsValidWithoutWhitespace(string value) + { + if (value == null) throw new ArgumentNullException(nameof(value)); + for (var ix = 0; ix < value.Length; ix++) + if (!Validator.ContainsKey(value[ix])) + return false; + return true; + } + } -} \ No newline at end of file + +} diff --git a/test/Solnet.Wallet.Test/KeysTest.cs b/test/Solnet.Wallet.Test/KeysTest.cs index 1feabafd..63451761 100644 --- a/test/Solnet.Wallet.Test/KeysTest.cs +++ b/test/Solnet.Wallet.Test/KeysTest.cs @@ -294,6 +294,8 @@ public void TestFindProgramAddress() public void TestIsValid() { Assert.IsTrue(PublicKey.IsValid("GUs5qLUfsEHkcMB9T38vjr18ypEhRuNWiePW2LoK4E3K")); + Assert.IsFalse(PublicKey.IsValid("GUs5qLUfsEHkcMB9T38vj*18ypEhRuNWiePW2LoK4E3K")); + Assert.IsFalse(PublicKey.IsValid("GUs5qLUfsEHkcMB9T38vjr18ypEhRuNWiePW2LoK4E3K ")); } [TestMethod] @@ -331,6 +333,7 @@ public void TestIsValid_False() public void TestIsValid_Empty_False() { Assert.IsFalse(PublicKey.IsValid("")); + Assert.IsFalse(PublicKey.IsValid(" ")); } [TestMethod] @@ -338,5 +341,20 @@ public void TestIsValid_InvalidB58_False() { Assert.IsFalse(PublicKey.IsValid("lllllll")); } + + [TestMethod] + [ExpectedException(typeof(ArgumentException))] + public void TestCreateBadPublicKeyFatal_1() + { + _ = new PublicKey("GUs5qLUfsEHkcMB9T38vjr18ypEhRuNWiePW2LoK4E3K "); + } + + [TestMethod] + [ExpectedException(typeof(ArgumentException))] + public void TestCreateBadPublicKeyFatal_2() + { + _ = new PublicKey("GUs5qLU&sEHkcMB9T38vjr18ypEhRuNWiePW2LoK4E3K"); + } + } } \ No newline at end of file From d7050632c7931ccaa6eeb2a016e1df02fb24ebde Mon Sep 17 00:00:00 2001 From: liquizard Date: Thu, 30 Jun 2022 17:19:45 +0100 Subject: [PATCH 2/4] Fix invalid public key arg in test --- test/Solnet.Extensions.Test/TokenWalletTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Solnet.Extensions.Test/TokenWalletTest.cs b/test/Solnet.Extensions.Test/TokenWalletTest.cs index e1116c11..5bbebaa0 100644 --- a/test/Solnet.Extensions.Test/TokenWalletTest.cs +++ b/test/Solnet.Extensions.Test/TokenWalletTest.cs @@ -284,7 +284,7 @@ public void TestTokenWalletSendAddressCheck() Assert.IsFalse(testTokenAccount.IsAssociatedTokenAccount); // trigger send to bogus target wallet - var targetOwner = "FAILzxtbcZ2vy3GSLLsZTEhbAqXPTRvEyoxa8wxSqKp5"; + var targetOwner = "BADxzxtbcZ2vy3GSLLsZTEhbAqXPTRvEyoxa8wxSqKp5"; wallet.Send(testTokenAccount, 1M, targetOwner, signer.PublicKey, builder => builder.Build(signer)); } From f136768b962dab40ddb9b126518ffcb288b78457 Mon Sep 17 00:00:00 2001 From: liquizard Date: Thu, 30 Jun 2022 17:20:07 +0100 Subject: [PATCH 3/4] Version bump --- SharedBuildProperties.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SharedBuildProperties.props b/SharedBuildProperties.props index b69d599a..a8811198 100644 --- a/SharedBuildProperties.props +++ b/SharedBuildProperties.props @@ -2,7 +2,7 @@ xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> Solnet - 6.0.13 + 6.0.14 Copyright 2022 © Solnet blockmountain blockmountain From 4dd603776ad3eb5178be238366ab19df53085a7b Mon Sep 17 00:00:00 2001 From: liquizard Date: Thu, 30 Jun 2022 21:05:58 +0100 Subject: [PATCH 4/4] Remove IsMaybeEncoded use IsValidWithoutWhitespace instead --- src/Solnet.Wallet/Utilities/Base58Encoder.cs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/Solnet.Wallet/Utilities/Base58Encoder.cs b/src/Solnet.Wallet/Utilities/Base58Encoder.cs index 46939ca5..9e1c44d4 100644 --- a/src/Solnet.Wallet/Utilities/Base58Encoder.cs +++ b/src/Solnet.Wallet/Utilities/Base58Encoder.cs @@ -39,17 +39,6 @@ public sealed class Base58Encoder : DataEncoder -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, }; - /// - /// Fast check if the string to know if base58 str - /// - /// - /// - public bool IsMaybeEncoded(string str) - { - bool maybeB58 = str.All(t => ((IList)PszBase58).Contains(t)); - - return maybeB58 && str.Length > 0; - } /// /// Encode the data.