From abfbdf6aec3bbf63a1f13e5706ee983efbfb4674 Mon Sep 17 00:00:00 2001
From: ccdle12 <chris.coverdale24@gmail.com>
Date: Wed, 27 Feb 2019 19:16:34 +0000
Subject: [PATCH] rpcserver: check for compatible network in SendCoins

lnd_test: adding address validation for send coins

The commit adds a test that checks that when a user calls sendcoins, the
receiving address is validated according to the current network. If the
address is not compatible with the current network, it will return an
error to the user.

rpcserver: adding a check for compatible network in SendCoins

This commit adds a check in SendCoins that checks whether the receiving
address is compatible with the current network.

Fixes #2677.
---
 lnd_test.go  | 32 ++++++++++++++++++++++++++++++--
 rpcserver.go | 27 ++++++++++++++++-----------
 2 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/lnd_test.go b/lnd_test.go
index e5057d85..3dd83c77 100644
--- a/lnd_test.go
+++ b/lnd_test.go
@@ -13168,12 +13168,40 @@ func testSweepAllCoins(net *lntest.NetworkHarness, t *harnessTest) {
 
 	// With the two coins above mined, we'll now instruct ainz to sweep all
 	// the coins to an external address not under its control.
+	// We will first attempt to send the coins to addresses that are not
+	// compatible with the current network. This is to test that the wallet
+	// will prevent any onchain transactions to addresses that are not on the
+	// same network as the user.
+
+	// Send coins to a testnet3 address.
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+	sweepReq := &lnrpc.SendCoinsRequest{
+		Addr:    "tb1qfc8fusa98jx8uvnhzavxccqlzvg749tvjw82tg",
+		SendAll: true,
+	}
+	_, err = ainz.SendCoins(ctxt, sweepReq)
+	if err == nil {
+		t.Fatalf("expected SendCoins to different network to fail")
+	}
+
+	// Send coins to a mainnet address.
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+	sweepReq = &lnrpc.SendCoinsRequest{
+		Addr:    "1MPaXKp5HhsLNjVSqaL7fChE3TVyrTMRT3",
+		SendAll: true,
+	}
+	_, err = ainz.SendCoins(ctxt, sweepReq)
+	if err == nil {
+		t.Fatalf("expected SendCoins to different network to fail")
+	}
+
+	// Send coins to a compatible address.
 	minerAddr, err := net.Miner.NewAddress()
 	if err != nil {
 		t.Fatalf("unable to create new miner addr: %v", err)
 	}
 
-	sweepReq := &lnrpc.SendCoinsRequest{
+	sweepReq = &lnrpc.SendCoinsRequest{
 		Addr:    minerAddr.String(),
 		SendAll: true,
 	}
@@ -13183,7 +13211,7 @@ func testSweepAllCoins(net *lntest.NetworkHarness, t *harnessTest) {
 		t.Fatalf("unable to sweep coins: %v", err)
 	}
 
-	// We'll mine a block wish should include the sweep transaction we
+	// We'll mine a block which should include the sweep transaction we
 	// generated above.
 	block := mineBlocks(t, net, 1, 1)[0]
 
diff --git a/rpcserver.go b/rpcserver.go
index e00d8bd1..6007e91c 100644
--- a/rpcserver.go
+++ b/rpcserver.go
@@ -880,6 +880,20 @@ func (r *rpcServer) SendCoins(ctx context.Context,
 		in.Addr, btcutil.Amount(in.Amount), int64(feePerKw),
 		in.SendAll)
 
+	// Decode the address receiving the coins, we need to check whether the
+	// address is valid for this network.
+	targetAddr, err := btcutil.DecodeAddress(in.Addr, activeNetParams.Params)
+	if err != nil {
+		return nil, err
+	}
+
+	// Make the check on the decoded address according to the active network.
+	if !targetAddr.IsForNet(activeNetParams.Params) {
+		return nil, fmt.Errorf("address: %v is not valid for this "+
+			"network: %v", targetAddr.String(),
+			activeNetParams.Params.Name)
+	}
+
 	var txid *chainhash.Hash
 
 	wallet := r.server.cc.wallet
@@ -896,15 +910,6 @@ func (r *rpcServer) SendCoins(ctx context.Context,
 				"active")
 		}
 
-		// Additionally, we'll need to convert the sweep address passed
-		// into a useable struct, and also query for the latest block
-		// height so we can properly construct the transaction.
-		sweepAddr, err := btcutil.DecodeAddress(
-			in.Addr, activeNetParams.Params,
-		)
-		if err != nil {
-			return nil, err
-		}
 		_, bestHeight, err := r.server.cc.chainIO.GetBestBlock()
 		if err != nil {
 			return nil, err
@@ -915,7 +920,7 @@ func (r *rpcServer) SendCoins(ctx context.Context,
 		// single transaction. This will be generated in a concurrent
 		// safe manner, so no need to worry about locking.
 		sweepTxPkg, err := sweep.CraftSweepAllTx(
-			feePerKw, uint32(bestHeight), sweepAddr, wallet,
+			feePerKw, uint32(bestHeight), targetAddr, wallet,
 			wallet.WalletController, wallet.WalletController,
 			r.server.cc.feeEstimator, r.server.cc.signer,
 		)
@@ -945,7 +950,7 @@ func (r *rpcServer) SendCoins(ctx context.Context,
 		// coin selection synchronization method to ensure that no coin
 		// selection (funding, sweep alls, other sends) can proceed
 		// while we instruct the wallet to send this transaction.
-		paymentMap := map[string]int64{in.Addr: in.Amount}
+		paymentMap := map[string]int64{targetAddr.String(): in.Amount}
 		err := wallet.WithCoinSelectLock(func() error {
 			newTXID, err := r.sendCoinsOnChain(paymentMap, feePerKw)
 			if err != nil {