Better error message on invalid PSBT in Sign with Seed (#6920)
What the developer wrote
Authored by Nicolas Dorier
Better error message on invalid PSBT in Sign with Seed (#6920)
* Better error message on invalid PSBT in Sign with Seed
* Improve error reporting if a PSBT built by SendWallet is not signable
What changed, and why it matters
This commit improves error messages shown to users when a Bitcoin transaction file (PSBT) cannot be signed because it is malformed or missing required data. It does not change the underlying signing rules; it only reports failures more clearly and redirects the user to an analysis view instead of attempting to sign an invalid PSBT. There is no direct evidence this fixes an exploitable vulnerability.
No urgent action required. Treat as routine hardening/UX improvement. Review whether the previous generic error could have led users to retry signing unsafe PSBTs, but no patch deployment priority is indicated.
Security signals we found
Improved error reporting for malformed PSBT inputs
Redirect from sign to analyze-psbt when PSBT is not ready to sign
Per-input sanity checks using GetSignableCoin and TryFinalizeInput
Evidence from the diff
The patch refactors PSBT validation in UIWalletsController. It replaces a generic ‘PSBT is not ready to be signed’ message with a detailed error builder (BuildErrorMessage) and per-input checks (GetSignableCoin / TryFinalizeInput). It also changes the SendWallet flow so that when command == ‘sign’ but the PSBT is not ready, it switches to ‘analyze-psbt’. This is a UX/hardening change rather than a security boundary fix.
Changed components
BTCPayServer/Controllers/UIWalletsController.PSBT.csBTCPayServer/Controllers/UIWalletsController.csWallet signing flow (Sign with Seed)SendWallet PSBT generation flowInspect captured patch +47 / −9
diff --git a/BTCPayServer/Controllers/UIWalletsController.PSBT.cs b/BTCPayServer/Controllers/UIWalletsController.PSBT.cs
index 79999c7..d5f3fb7 100644
--- a/BTCPayServer/Controllers/UIWalletsController.PSBT.cs
+++ b/BTCPayServer/Controllers/UIWalletsController.PSBT.cs
@@ -86,6 +86,7 @@ namespace BTCPayServer.Controllers
BackUrl = vm.BackUrl
});
}
+
switch (command)
{
case "vault":
@@ -170,6 +171,7 @@ namespace BTCPayServer.Controllers
{
return View("WalletPSBT", vm);
}
+
switch (command)
{
case "createpending":
@@ -373,14 +375,25 @@ namespace BTCPayServer.Controllers
vm.FeeRate = feeRate.ToString();
}
- var sanityErrors = psbtObject.CheckSanity();
- if (sanityErrors.Count != 0)
- {
- vm.SetErrors(sanityErrors);
- }
- else if (!psbtObject.IsAllFinalized() && !psbtObject.TryFinalize(out var errors))
+ if (!psbtObject.IsAllFinalized())
{
- vm.SetErrors(errors);
+ var sanityErrors = new List<PSBTError>();
+ foreach (var input in psbtObject.Inputs)
+ {
+ if (input.IsFinalized())
+ continue;
+
+ if (input.GetSignableCoin(out var missingCoin) is null)
+ {
+ sanityErrors.Add(new PSBTError(input.Index, missingCoin));
+ }
+ else if (!input.TryFinalizeInput(out var err))
+ {
+ sanityErrors.Add(err[0]);
+ }
+ }
+ if (sanityErrors.Count > 0)
+ vm.SetErrors(sanityErrors);
}
var combinedTypeIds = inputToObjects.Values.SelectMany(ids => ids).Concat(outputToObjects.Values)
diff --git a/BTCPayServer/Controllers/UIWalletsController.cs b/BTCPayServer/Controllers/UIWalletsController.cs
index e2a3537..68c61bd 100644
--- a/BTCPayServer/Controllers/UIWalletsController.cs
+++ b/BTCPayServer/Controllers/UIWalletsController.cs
@@ -4,6 +4,7 @@ using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Net.Mime;
+using System.Text;
using System.Threading;
using System.Threading.Tasks;
using BTCPayServer.Abstractions.Constants;
@@ -1289,6 +1290,9 @@ namespace BTCPayServer.Controllers
ChangeAddress = psbtResponse.ChangeAddress?.ToString(),
PSBT = psbt.ToHex()
};
+
+ if (!psbt.IsReadyToSign() && command == "sign")
+ command = "analyze-psbt";
switch (command)
{
case "createpending":
@@ -1512,9 +1516,9 @@ namespace BTCPayServer.Controllers
var psbt = PSBT.Parse(viewModel.SigningContext.PSBT, network.NBitcoinNetwork);
- if (!psbt.IsReadyToSign())
+ if (!psbt.IsReadyToSign(out var errors))
{
- ModelState.AddModelError(nameof(viewModel.SigningContext.PSBT), "PSBT is not ready to be signed");
+ ModelState.AddModelError(nameof(viewModel.SigningContext.PSBT), BuildErrorMessage(errors));
}
if (!ModelState.IsValid)
@@ -1580,6 +1584,27 @@ namespace BTCPayServer.Controllers
});
}
+ private static string BuildErrorMessage(PSBTError[] errors)
+ {
+ StringBuilder errorMessage = new();
+ errorMessage.Append("PSBT is not ready to be signed.");
+ if (errors.Length == 1)
+ {
+ errorMessage.Append($" ({errors[0]})");
+ }
+ else
+ {
+ errorMessage.AppendLine();
+ foreach (var error in errors.Take(5))
+ {
+ errorMessage.AppendLine(error.ToString());
+ }
+ }
+ if (errors.Length > 5)
+ errorMessage.Append($"{errors.Length - 5} more errors...");
+ return errorMessage.ToString();
+ }
+
private WalletPSBTReadyViewModel.StringAmounts ValueToString(Money v, BTCPayNetworkBase network,
FiatRate? rate) =>
new(
Why this scored 23/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.