From 8e927eee736e0ff0e9762c243a2bb5fcfa5951c8 Mon Sep 17 00:00:00 2001 From: "nicolas.dorier" Date: Tue, 21 Jan 2025 20:26:44 +0900 Subject: [PATCH] Better validation of inputs when setting on-chain payment method by API --- BTCPayServer.Tests/GreenfieldAPITests.cs | 7 ++- .../Controllers/UIStoresController.Onchain.cs | 13 ++--- BTCPayServer/DerivationSchemeSettings.cs | 7 ++- .../Bitcoin/BitcoinLikePaymentHandler.cs | 53 +++++++++++++++++-- ...agger.template.stores-payment-methods.json | 10 ++-- 5 files changed, 74 insertions(+), 16 deletions(-) diff --git a/BTCPayServer.Tests/GreenfieldAPITests.cs b/BTCPayServer.Tests/GreenfieldAPITests.cs index 70e19d20b..330a1c15d 100644 --- a/BTCPayServer.Tests/GreenfieldAPITests.cs +++ b/BTCPayServer.Tests/GreenfieldAPITests.cs @@ -3314,6 +3314,12 @@ namespace BTCPayServer.Tests await viewOnlyClient.SendHttpRequest($"api/v1/stores/{store.Id}/payment-methods/onchain/BTC/preview", new JObject() { ["config"] = xpub.ToString() }, HttpMethod.Post); var method = await client.UpdateStorePaymentMethod(store.Id, "BTC-CHAIN", new UpdatePaymentMethodRequest() { Enabled = true, Config = JValue.CreateString(xpub.ToString())}); + var method2 = await client.UpdateStorePaymentMethod(store.Id, "BTC-CHAIN", new UpdatePaymentMethodRequest() { Enabled = true, Config = new JObject() { ["derivationScheme"] = xpub.ToString(), ["label"] = "test", ["accountKeyPath"] = "aaaaaaaa/84'/0'/0'" } }); + Assert.Equal("aaaaaaaa", method2.Config["accountKeySettings"][0]["rootFingerprint"].ToString()); + Assert.Equal("84'/0'/0'", method2.Config["accountKeySettings"][0]["accountKeyPath"].ToString()); + var method3 = await client.UpdateStorePaymentMethod(store.Id, "BTC-CHAIN", new UpdatePaymentMethodRequest() { Enabled = true, Config = new JObject() { ["derivationScheme"] = xpub.ToString() } }); + + Assert.Equal(method.ToJson(), method3.ToJson()); Assert.Equal(firstAddress, (await viewOnlyClient.PreviewStoreOnChainPaymentMethodAddresses(store.Id, "BTC")).Addresses.First().Address); await AssertHttpError(403, async () => @@ -3378,7 +3384,6 @@ namespace BTCPayServer.Tests Assert.Equal(24, generateResponse.Mnemonic.Words.Length); Assert.Equal(Wordlist.Japanese, generateResponse.Mnemonic.WordList); - } [Fact(Timeout = 60 * 2 * 1000)] diff --git a/BTCPayServer/Controllers/UIStoresController.Onchain.cs b/BTCPayServer/Controllers/UIStoresController.Onchain.cs index 06d287929..ee20989d7 100644 --- a/BTCPayServer/Controllers/UIStoresController.Onchain.cs +++ b/BTCPayServer/Controllers/UIStoresController.Onchain.cs @@ -408,6 +408,7 @@ public partial class UIStoresController var client = _explorerProvider.GetExplorerClient(network); var handler = _handlers.GetBitcoinHandler(cryptoCode); + var vm = new WalletSettingsViewModel { StoreId = storeId, @@ -417,18 +418,18 @@ public partial class UIStoresController Network = network, IsHotWallet = derivation.IsHotWallet, Source = derivation.Source, - RootFingerprint = derivation.GetSigningAccountKeySettings().RootFingerprint.ToString(), - DerivationScheme = derivation.AccountDerivation.ToString(), + RootFingerprint = derivation.GetSigningAccountKeySettingsOrDefault()?.RootFingerprint.ToString(), + DerivationScheme = derivation.AccountDerivation?.ToString(), DerivationSchemeInput = derivation.AccountOriginal, - KeyPath = derivation.GetSigningAccountKeySettings().AccountKeyPath?.ToString(), + KeyPath = derivation.GetSigningAccountKeySettingsOrDefault()?.AccountKeyPath?.ToString(), UriScheme = network.NBitcoinNetwork.UriScheme, Label = derivation.Label, - SelectedSigningKey = derivation.SigningKey.ToString(), + SelectedSigningKey = derivation.SigningKey?.ToString(), NBXSeedAvailable = derivation.IsHotWallet && canUseHotWallet && !string.IsNullOrEmpty(await client.GetMetadataAsync(derivation.AccountDerivation, WellknownMetadataKeys.MasterHDKey)), - AccountKeys = derivation.AccountKeySettings + AccountKeys = (derivation.AccountKeySettings ?? []) .Select(e => new WalletSettingsAccountKeyViewModel { AccountKey = e.AccountKey.ToString(), @@ -441,7 +442,7 @@ public partial class UIStoresController CanUseHotWallet = canUseHotWallet, CanUseRPCImport = rpcImport, StoreName = store.StoreName, - CanSetupMultiSig = derivation.AccountKeySettings.Length > 1, + CanSetupMultiSig = (derivation.AccountKeySettings ?? []).Length > 1, IsMultiSigOnServer = derivation.IsMultiSigOnServer, DefaultIncludeNonWitnessUtxo = derivation.DefaultIncludeNonWitnessUtxo }; diff --git a/BTCPayServer/DerivationSchemeSettings.cs b/BTCPayServer/DerivationSchemeSettings.cs index fd57e421d..cbe38d25a 100644 --- a/BTCPayServer/DerivationSchemeSettings.cs +++ b/BTCPayServer/DerivationSchemeSettings.cs @@ -80,7 +80,12 @@ namespace BTCPayServer public AccountKeySettings GetSigningAccountKeySettings() { - return AccountKeySettings.Single(a => a.AccountKey == SigningKey); + return (AccountKeySettings ?? []).Single(a => a.AccountKey == SigningKey); + } + + public AccountKeySettings GetSigningAccountKeySettingsOrDefault() + { + return (AccountKeySettings ?? []).SingleOrDefault(a => a.AccountKey == SigningKey); } public AccountKeySettings[] AccountKeySettings { get; set; } diff --git a/BTCPayServer/Payments/Bitcoin/BitcoinLikePaymentHandler.cs b/BTCPayServer/Payments/Bitcoin/BitcoinLikePaymentHandler.cs index 33dfd314f..b3b347f27 100644 --- a/BTCPayServer/Payments/Bitcoin/BitcoinLikePaymentHandler.cs +++ b/BTCPayServer/Payments/Bitcoin/BitcoinLikePaymentHandler.cs @@ -23,6 +23,7 @@ using NBXplorer.DerivationStrategy; using NBXplorer.Models; using Newtonsoft.Json; using Newtonsoft.Json.Linq; +using static Org.BouncyCastle.Math.EC.ECCurve; using StoreData = BTCPayServer.Data.StoreData; namespace BTCPayServer.Payments.Bitcoin @@ -39,7 +40,7 @@ namespace BTCPayServer.Payments.Bitcoin private readonly NBXplorerDashboard _dashboard; private readonly WalletRepository _walletRepository; private readonly Services.Wallets.BTCPayWalletProvider _WalletProvider; - + public JsonSerializer Serializer { get; } public PaymentMethodId PaymentMethodId { get; private set; } public BTCPayNetwork Network => _Network; @@ -233,16 +234,50 @@ namespace BTCPayServer.Payments.Bitcoin return null; return GetAccountDerivation(value, network); } + class AlternativeConfig + { + [JsonProperty] + public DerivationStrategyBase DerivationScheme { get; set; } + [JsonProperty] + public string Label { get; set; } + [JsonProperty] + public RootedKeyPath AccountKeyPath { get; set; } + + public DerivationSchemeSettings ToDerivationSchemeSettings(BTCPayNetwork network) + { + if (DerivationScheme is null) + return null; + var scheme = new DerivationSchemeSettings(DerivationScheme, network); + scheme.AccountOriginal = DerivationScheme.ToString(); + scheme.Label = Label; + if (AccountKeyPath is not null && scheme.AccountKeySettings is [{ } ak, ..]) + { + ak.RootFingerprint = AccountKeyPath.MasterFingerprint; + ak.AccountKeyPath = AccountKeyPath.KeyPath; + } + return scheme; + } + } public Task ValidatePaymentMethodConfig(PaymentMethodConfigValidationContext validationContext) { var parser = Network.GetDerivationSchemeParser(); DerivationSchemeSettings settings = new DerivationSchemeSettings(); - if (parser.TryParseXpub(validationContext.Config.ToString(), ref settings)) + if (validationContext.Config is JValue { Type: JTokenType.String, Value: string config } + && parser.TryParseXpub(config, ref settings)) { validationContext.Config = JToken.FromObject(settings, Serializer); return Task.CompletedTask; } - var res = validationContext.Config.ToObject(Serializer); + DerivationSchemeSettings res = null; + try + { + res = validationContext.Config.ToObject(Serializer)?.ToDerivationSchemeSettings(Network); + if (res != null) + validationContext.Config = JToken.FromObject(res, Serializer); + } + catch { } + + res ??= validationContext.Config.ToObject(Serializer); if (res is null) { validationContext.ModelState.AddModelError(nameof(validationContext.Config), "Invalid derivation scheme settings"); @@ -252,6 +287,18 @@ namespace BTCPayServer.Payments.Bitcoin { validationContext.ModelState.AddModelError(nameof(res.AccountDerivation), "Invalid account derivation"); } + if (res.AccountKeySettings is null) + { + validationContext.ModelState.AddModelError(nameof(res.AccountKeySettings), "Invalid AccountKeySettings"); + } + if (res.SigningKey is null) + { + validationContext.ModelState.AddModelError(nameof(res.SigningKey), "Invalid SigningKey"); + } + if (res.GetSigningAccountKeySettingsOrDefault() is null) + { + validationContext.ModelState.AddModelError(nameof(res.AccountKeySettings), "AccountKeySettings doesn't include the SigningKey"); + } return Task.CompletedTask; } diff --git a/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-payment-methods.json b/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-payment-methods.json index e3bb0269b..1c10df3eb 100644 --- a/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-payment-methods.json +++ b/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-payment-methods.json @@ -3,7 +3,7 @@ "/api/v1/stores/{storeId}/payment-methods": { "get": { "tags": [ - "Store Payment Methods" + "Store (Payment Methods)" ], "summary": "Get store payment methods", "description": "View information about the stores' configured payment methods", @@ -79,7 +79,7 @@ "/api/v1/stores/{storeId}/payment-methods/{paymentMethodId}": { "get": { "tags": [ - "Store Payment Methods" + "Store (Payment Methods)" ], "summary": "Get store payment method", "description": "View information about the stores' configured payment method", @@ -144,7 +144,7 @@ }, "put": { "tags": [ - "Store Payment Methods" + "Store (Payment Methods)" ], "summary": "Update store's payment method", "description": "Update information about the stores' configured payment method", @@ -209,7 +209,7 @@ }, "delete": { "tags": [ - "Store Payment Methods" + "Store (Payment Methods)" ], "summary": "Delete store's payment method", "description": "Delete information about the stores' configured payment method", @@ -369,7 +369,7 @@ }, "tags": [ { - "name": "Store Payment Methods", + "name": "Store (Payment Methods)", "description": "Store Payment Methods operations" } ]