From 1ca1d77bf7f8b968c4a53f8bd7eac303fda5f2eb Mon Sep 17 00:00:00 2001
From: Andrew Kozlik <andrew.kozlik@gmail.com>
Date: Thu, 17 Oct 2019 11:06:23 +0200
Subject: [PATCH] tests/sign_tx: Add a test to ensure that if the change output
 is modified after the user confirms the transaction, then signing fails.

---
 tests/device_tests/test_msg_signtx.py      | 53 ++++++++++++++++++++++
 tests/device_tests/test_multisig_change.py | 18 ++++----
 2 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/tests/device_tests/test_msg_signtx.py b/tests/device_tests/test_msg_signtx.py
index 22c2cca286..6d3414e634 100644
--- a/tests/device_tests/test_msg_signtx.py
+++ b/tests/device_tests/test_msg_signtx.py
@@ -615,6 +615,59 @@ class TestMsgSigntx:
         )
         assert exc.value.args[1].endswith("Transaction has changed during signing")
 
+    # Ensure that if the change output is modified after the user confirms the
+    # transaction, then signing fails.
+    def test_attack_modify_change_address(self, client):
+        # see 87be0736f202f7c2bff0781b42bad3e0cdcb54761939da69ea793a3735552c56
+
+        # tx: e5040e1bc1ae7667ffb9e5248e90b2fb93cd9150234151ce90e14ab2f5933bcd
+        # input 0: 0.31 BTC
+        inp1 = proto.TxInputType(
+            address_n=parse_path("44'/1'/0'/0/0"),
+            # amount=31000000,
+            prev_hash=TXHASH_e5040e,
+            prev_index=0,
+        )
+
+        out1 = proto.TxOutputType(
+            address="msj42CCGruhRsFrGATiUuh25dtxYtnpbTx",
+            amount=30090000,
+            script_type=proto.OutputScriptType.PAYTOADDRESS,
+        )
+
+        out2 = proto.TxOutputType(
+            address_n=parse_path("44'/1'/0'/1/0"),
+            amount=900000,
+            script_type=proto.OutputScriptType.PAYTOADDRESS,
+        )
+
+        run_attack = False
+
+        def attack_processor(msg):
+            nonlocal run_attack
+            if msg.tx.outputs and msg.tx.outputs[0] == out2:
+                if not run_attack:
+                    run_attack = True
+                else:
+                    msg.tx.outputs[0].address_n = []
+                    msg.tx.outputs[0].address = "mwue7mokpBRAsJtHqEMcRPanYBmsSmYKvY"
+
+            return msg
+
+        # Set up attack processors
+        client.set_filter(proto.TxAck, attack_processor)
+
+        with pytest.raises(CallException) as exc:
+            btc.sign_tx(
+                client, "Testnet", [inp1], [out1, out2], prev_txes=tx_cache("Testnet")
+            )
+
+        assert exc.value.args[0] in (
+            proto.FailureType.ProcessError,
+            proto.FailureType.DataError,
+        )
+        assert exc.value.args[1].endswith("Transaction has changed during signing")
+
     def test_attack_change_input_address(self, client):
         inp1 = proto.TxInputType(
             address_n=parse_path("44'/1'/4'/0/0"),
diff --git a/tests/device_tests/test_multisig_change.py b/tests/device_tests/test_multisig_change.py
index cd76cc6c0f..33db517d75 100644
--- a/tests/device_tests/test_multisig_change.py
+++ b/tests/device_tests/test_multisig_change.py
@@ -52,21 +52,21 @@ class TestMultisigChange:
 
     # ext1 + ext2 + int
     #   redeemscript (2 of 3): 522102c0d0c5fee952620757c6128dbf327c996cd72ed3358d15d6518a1186099bc15e210388460dc439f4c8f5bcfc268c36e11b4375cad5c3535c336cfdf8c32c3afad5c1210338d78612e990f2eea0c426b5e48a8db70b9d7ed66282b3b26511e0b1c75515a653ae
-    #   multisig address: 3Gj7y1FdTppx2JEDqYqAEZFnKCA4GRysKF
-    #   tx: d1d08ea63255af4ad16b098e9885a252632086fa6be53301521d05253ce8a73d
-    #   input 0: 0.001 BTC
+    #   multisig address: 2N9W4z9AhAPaHghtqVQPbaTAGHdbrhKeBQw
+    #   tx: 16c6c8471b8db7a628f2b2bb86bfeefae1766463ce8692438c7fd3fce3f43ce5
+    #   input 0: 0.5 BTC
 
     # ext1 + int + ext2
     #   redeemscript (2 of 3): 522102c0d0c5fee952620757c6128dbf327c996cd72ed3358d15d6518a1186099bc15e210338d78612e990f2eea0c426b5e48a8db70b9d7ed66282b3b26511e0b1c75515a6210388460dc439f4c8f5bcfc268c36e11b4375cad5c3535c336cfdf8c32c3afad5c153ae
-    #   multisig address: 3QsvfB6d1LzYcpm8xyhS1N1HBRrzHTgLHB
-    #   tx: a6e2829d089cee47e481b1a753a53081b40738cc87e38f1d9b23ab57d9ad4396
-    #   input 0: 0.001 BTC
+    #   multisig address: 2NDBG6QXQLtnQ3jRGkrqo53BiCeXfQXLdj4
+    #   tx: d80c34ee14143a8bf61125102b7ef594118a3796cad670fa8ee15080ae155318
+    #   input 0: 0.345 BTC
 
     # ext1 + ext3 + int
     #   redeemscript (2 of 3): 522102c0d0c5fee952620757c6128dbf327c996cd72ed3358d15d6518a1186099bc15e2102e0c21e2a7cf00b94c5421725acff97f9826598b91f2340c5ddda730caca7d648210338d78612e990f2eea0c426b5e48a8db70b9d7ed66282b3b26511e0b1c75515a653ae
-    #   multisig address: 37LvC1Q5CyKbMbKMncEJdXxqGhHxrBEgPE
-    #   tx: e4bc1ae5e5007a08f2b3926fe11c66612e8f73c6b00c69c7027213b84d259be3
-    #   input 1: 0.001 BTC
+    #   multisig address: 2MvwPWfp2XPU3S1cMwgEMKBPUw38VP5SBE4
+    #   tx: b0946dc27ba308a749b11afecc2018980af18f79e89ad6b080b58220d856f739
+    #   input 1: 0.555 BTC
 
     multisig_in1 = proto.MultisigRedeemScriptType(
         nodes=[node_ext2, node_ext1, node_int],