From 681e3ceedebb1769e9b6a6d71a7a3c54f89b2a3e Mon Sep 17 00:00:00 2001 From: Torkel Rogstad Date: Fri, 22 Apr 2022 22:03:08 -0400 Subject: [PATCH] rpc: verify address is for correct net Verify that the addresses we're decoding when sending coins onchain are for the correct network. Without this check we'll convert the users addresses to their equivalent on other networks, which is a gross violation of the principle of least astonishment. --- docs/release-notes/release-notes-0.17.0.md | 3 +++ lnrpc/invoicesrpc/addinvoice.go | 11 +++++++++-- lnrpc/walletrpc/walletkit_server.go | 6 ++++++ rpcserver.go | 14 +++++++++++++- 4 files changed, 31 insertions(+), 3 deletions(-) diff --git a/docs/release-notes/release-notes-0.17.0.md b/docs/release-notes/release-notes-0.17.0.md index 63af5901e..f1d443a6a 100644 --- a/docs/release-notes/release-notes-0.17.0.md +++ b/docs/release-notes/release-notes-0.17.0.md @@ -72,6 +72,9 @@ package](https://github.com/lightningnetwork/lnd/pull/7356) `grpc.keepalive_time_ms=5100` is recommended on the client side (adding 100ms to account for slightly different clock speeds). +* [Fixed a bug where we didn't check for correct networks when submitting + onchain transactions](https://github.com/lightningnetwork/lnd/pull/6448). + ## Misc * [Generate default macaroons diff --git a/lnrpc/invoicesrpc/addinvoice.go b/lnrpc/invoicesrpc/addinvoice.go index 745952147..7c59da883 100644 --- a/lnrpc/invoicesrpc/addinvoice.go +++ b/lnrpc/invoicesrpc/addinvoice.go @@ -292,12 +292,19 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig, // If specified, add a fallback address to the payment request. if len(invoice.FallbackAddr) > 0 { - addr, err := btcutil.DecodeAddress(invoice.FallbackAddr, - cfg.ChainParams) + addr, err := btcutil.DecodeAddress( + invoice.FallbackAddr, cfg.ChainParams, + ) if err != nil { return nil, nil, fmt.Errorf("invalid fallback "+ "address: %v", err) } + + if !addr.IsForNet(cfg.ChainParams) { + return nil, nil, fmt.Errorf("fallback address is not "+ + "for %s", cfg.ChainParams.Name) + } + options = append(options, zpay32.FallbackAddr(addr)) } diff --git a/lnrpc/walletrpc/walletkit_server.go b/lnrpc/walletrpc/walletkit_server.go index ded945bb7..9b6974fd7 100644 --- a/lnrpc/walletrpc/walletkit_server.go +++ b/lnrpc/walletrpc/walletkit_server.go @@ -1104,6 +1104,12 @@ func (w *WalletKit) FundPsbt(_ context.Context, "%s for network %s: %v", addrStr, w.cfg.ChainParams.Name, err) } + + if !addr.IsForNet(w.cfg.ChainParams) { + return nil, fmt.Errorf("address is not for %s", + w.cfg.ChainParams.Name) + } + pkScript, err := txscript.PayToAddrScript(addr) if err != nil { return nil, fmt.Errorf("error getting pk "+ diff --git a/rpcserver.go b/rpcserver.go index 56963fb84..265bce77a 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -970,6 +970,11 @@ func addrPairsToOutputs(addrPairs map[string]int64, return nil, err } + if !addr.IsForNet(params) { + return nil, fmt.Errorf("address is not for %s", + params.Name) + } + pkscript, err := txscript.PayToAddrScript(addr) if err != nil { return nil, err @@ -2579,7 +2584,14 @@ func (r *rpcServer) CloseChannel(in *lnrpc.CloseChannelRequest, in.DeliveryAddress, r.cfg.ActiveNetParams.Params, ) if err != nil { - return fmt.Errorf("invalid delivery address: %v", err) + return fmt.Errorf("invalid delivery address: "+ + "%v", err) + } + + if !addr.IsForNet(r.cfg.ActiveNetParams.Params) { + return fmt.Errorf("delivery address is not "+ + "for %s", + r.cfg.ActiveNetParams.Params.Name) } // Create a script to pay out to the address provided.