core: add hard limit for transaction fees

The hard limit is set to 10*fee_warning_threshold. The limit is not
enforced when `safety_checks` is set to "Prompt".

core/CHANGELOG.md

@@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ### Added
 - Running the frozen version of the emulator doesn't need arguments.  [#1115]
 - CoinJoin preauthorization and siging flow.  [#1053]
+- Hard limit on transaction fees. Can be disabled using `safety-checks`. [#1087]
 
 ### Changed
 - Print inverted question mark for non-printable characters.
@@ -254,6 +255,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 [#1056]: https://github.com/trezor/trezor-firmware/issues/1056
 [#1067]: https://github.com/trezor/trezor-firmware/issues/1067
 [#1074]: https://github.com/trezor/trezor-firmware/issues/1074
+[#1087]: https://github.com/trezor/trezor-firmware/issues/1087
 [#1089]: https://github.com/trezor/trezor-firmware/issues/1089
 [#1098]: https://github.com/trezor/trezor-firmware/issues/1098
 [#1115]: https://github.com/trezor/trezor-firmware/issues/1115

core/src/apps/bitcoin/sign_tx/approvers.py

@@ -1,5 +1,6 @@
 from micropython import const
 
+from storage import device
 from trezor import wire
 from trezor.messages.SignTx import SignTx
 from trezor.messages.TxInputType import TxInputType
@@ -88,9 +89,13 @@ async def approve_tx(self) -> None:
 
         total = self.total_in - self.change_out
         spending = total - self.external_in
+        # fee_threshold = (coin.maxfee per byte * tx size)
+        fee_threshold = (self.coin.maxfee_kb / 1000) * (self.weight.get_total() / 4)
 
         # fee > (coin.maxfee per byte * tx size)
-        if fee > (self.coin.maxfee_kb / 1000) * (self.weight.get_total() / 4):
+        if fee > fee_threshold:
+            if fee > 10 * fee_threshold and not device.unsafe_prompts_allowed():
+                raise wire.DataError("The fee is unexpectedly large")
             await helpers.confirm_feeoverthreshold(fee, self.coin)
         if self.change_count > self.MAX_SILENT_CHANGE_COUNT:
             await helpers.confirm_change_count_over_threshold(self.change_count)

tests/device_tests/test_msg_signtx.py

@@ -16,7 +16,7 @@
 
 import pytest
 
-from trezorlib import btc, messages
+from trezorlib import btc, device, messages
 from trezorlib.exceptions import TrezorFailure
 from trezorlib.tools import H_, parse_path, tx_hash
 
@@ -171,7 +171,7 @@ def test_testnet_one_two_fee(self, client):
             == "0100000001cd3b93f5b24ae190ce5141235091cd93fbb2908e24e5b9ff6776aec11b0e04e5000000006b483045022100eba3bbcbb82ab1ebac88a394e8fb53b0263dadbb3e8072f0a21ee62818c911060220686a9b7f306d028b54a228b5c47cc6c27b1d01a3b0770440bcc64d55d8bace2c0121030e669acac1f280d1ddf441cd2ba5e97417bf2689e4bbec86df4f831bf9f7ffd0ffffffff021023cb01000000001976a91485eb47fe98f349065d6f044e27a4ac541af79ee288aca0bb0d00000000001976a9143d3cca567e00a04819742b21a696a67da796498b88ac00000000"
         )
 
-    def test_testnet_fee_too_high(self, client):
+    def test_testnet_fee_high_warning(self, client):
         # tx: 6f90f3c7cbec2258b0971056ef3fe34128dbde30daa9c0639a898f9977299d54
         # input 1: 10.00000000 BTC
         inp1 = messages.TxInputType(
@@ -183,7 +183,7 @@ def test_testnet_fee_too_high(self, client):
 
         out1 = messages.TxOutputType(
             address="mfiGQVPcRcaEvQPYDErR34DcCovtxYvUUV",
-            amount=1000000000 - 500000000 - 100000000,
+            amount=1000000000 - 500000000 - 8000000,
             script_type=messages.OutputScriptType.PAYTOADDRESS,
         )
 
@@ -221,7 +221,7 @@ def test_testnet_fee_too_high(self, client):
 
         assert (
             tx_hash(serialized_tx).hex()
-            == "c669527e0d80dc645925f6965e1622e71fa5ca51e284442c4620c1ade7a76c63"
+            == "54fd5e9b65b8acc10144c1e78ea9720df7606d7d4a543e4c547ecd45b2ae226b"
         )
 
     def test_one_two_fee(self, client):
@@ -577,7 +577,7 @@ def test_lots_of_change(self, client):
             == "fae68e4a3a4b0540eb200e2218a6d8465eac469788ccb236e0d5822d105ddde9"
         )
 
-    def test_fee_too_high(self, client):
+    def test_fee_high_warning(self, client):
         # tx: 1570416eb4302cf52979afd5e6909e37d8fdd874301f7cc87e547e509cb1caa6
         # input 0: 1.0 BTC
 
@@ -621,6 +621,52 @@ def test_fee_too_high(self, client):
             == "c36928aca6452d50cb63e2592200bbcc3722ce6b631b1dfd185ccdf9a954af28"
         )
 
+    @pytest.mark.skip_t1
+    def test_fee_high_hardfail(self, client):
+        # tx: 1570416eb4302cf52979afd5e6909e37d8fdd874301f7cc87e547e509cb1caa6
+        # input 0: 1.0 BTC
+
+        inp1 = messages.TxInputType(
+            address_n=parse_path("44h/0h/0h/0/0"),
+            # amount=100000000,
+            prev_hash=TXHASH_157041,
+            prev_index=0,
+        )
+
+        out1 = messages.TxOutputType(
+            address="1MJ2tj2ThBE62zXbBYA5ZaN3fdve5CPAz1",
+            amount=100000000 - 5100000,
+            script_type=messages.OutputScriptType.PAYTOADDRESS,
+        )
+
+        with pytest.raises(TrezorFailure, match="fee is unexpectedly large"):
+            btc.sign_tx(client, "Bitcoin", [inp1], [out1], prev_txes=TX_CACHE_MAINNET)
+
+        # set SafetyCheckLevel to Prompt and try again
+        device.apply_settings(client, safety_checks=messages.SafetyCheckLevel.Prompt)
+        with client:
+            finished = False
+
+            def input_flow():
+                nonlocal finished
+                for expected in (B.ConfirmOutput, B.FeeOverThreshold, B.SignTx):
+                    br = yield
+                    assert br == expected
+                    client.debug.press_yes()
+                finished = True
+
+            client.set_input_flow(input_flow)
+
+            _, serialized_tx = btc.sign_tx(
+                client, "Bitcoin", [inp1], [out1], prev_txes=TX_CACHE_MAINNET
+            )
+            assert finished
+
+        assert (
+            tx_hash(serialized_tx).hex()
+            == "0fadc325662e84fd1a5efcb20c5369cf9134a24b6d29bce99f61e69680397a79"
+        )
+
     def test_not_enough_funds(self, client):
         # tx: d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882
         # input 0: 0.0039 BTC

tests/device_tests/test_msg_signtx_prevhash.py

@@ -152,7 +152,7 @@ def test_invalid_prev_hash_in_prevtx(client, prev_hash):
     )
     out1 = messages.TxOutputType(
         address="1MJ2tj2ThBE62zXbBYA5ZaN3fdve5CPAz1",
-        amount=1000,
+        amount=99000000,
         script_type=messages.OutputScriptType.PAYTOADDRESS,
     )
     btc.sign_tx(client, "Bitcoin", [inp0], [out1], prev_txes={tx_hash: prev_tx})

tests/device_tests/test_msg_signtx_segwit.py

@@ -362,7 +362,7 @@ def test_attack_mixed_inputs(self, client):
         )
         out1 = proto.TxOutputType(
             address="mhRx1CeVfaayqRwq5zgRQmD7W5aWBfD5mC",
-            amount=30998000,
+            amount=31000000 + TRUE_AMOUNT - 3456789,
             script_type=proto.OutputScriptType.PAYTOADDRESS,
         )
 

tests/ui_tests/fixtures.json

@@ -286,7 +286,8 @@
 "test_msg_signtx.py-test_attack_change_outputs": "4872e0db49b2c66f2f033d055abc086520cdd667ffe48ead0ad5ed0f4452af1a",
 "test_msg_signtx.py-test_attack_modify_change_address": "cfd5c83510c044c456622298138e222aee135a6df607bb6e5603228535f0762f",
 "test_msg_signtx.py-test_change_on_main_chain_allowed": "cfd5c83510c044c456622298138e222aee135a6df607bb6e5603228535f0762f",
-"test_msg_signtx.py-test_fee_too_high": "8cb3b31dce25fa36cd5c8322c71611dc7bc9d2290579ffd88dd67d21058bde04",
+"test_msg_signtx.py-test_fee_high_hardfail": "b450a59808fb20cbd01d34e8d24bf1a5814e9b2a10109710240c617b68e247b6",
+"test_msg_signtx.py-test_fee_high_warning": "8cb3b31dce25fa36cd5c8322c71611dc7bc9d2290579ffd88dd67d21058bde04",
 "test_msg_signtx.py-test_lots_of_change": "9e143458b399d187b6a3060fc95b998822f5a7ed67d6915610fd02c0ccab791e",
 "test_msg_signtx.py-test_not_enough_funds": "dbaa027aa1f4b08b138a5965245593dab2a662b0f4d88dd28b82a64f88f5d7fe",
 "test_msg_signtx.py-test_one_one_fee": "f6b6662fa1384f20640522a169575f8ca26185fca8ca3bc2a3a5ccd1fa9d2f68",
@@ -295,7 +296,7 @@
 "test_msg_signtx.py-test_p2sh": "bac5ead8e28a6439c8f961f07e7d27c3fa82d3dfdbb351640b6f70bb0e1644a5",
 "test_msg_signtx.py-test_spend_coinbase": "f498aec01de57978d14dd2e02bae2a3d904a1802e06f15be4ed181976af5130d",
 "test_msg_signtx.py-test_testnet_big_amount": "5b84d787542e5fa1436db4e768fbac15f92662a6a0deb580012def5a788adf12",
-"test_msg_signtx.py-test_testnet_fee_too_high": "97f1650cd03286b305db65b6aa764ba9029b53f6cb0dd334d3b866c1297136d2",
+"test_msg_signtx.py-test_testnet_fee_high_warning": "7ae33df914ce2025a3eb26a6a6ad915cd8891b5070cf3318ed1a279288a01251",
 "test_msg_signtx.py-test_testnet_one_two_fee": "cfd5c83510c044c456622298138e222aee135a6df607bb6e5603228535f0762f",
 "test_msg_signtx.py-test_two_changes": "832d4b168551c37c9e09cf2ce16fd062d6599bcd0473305f70c5175bd874a920",
 "test_msg_signtx.py-test_two_two": "57707ecbcb77f670148c6076724b3da2e880d27ecf86e29135af4a5aeef6fdbc",
@@ -338,7 +339,7 @@
 "test_msg_signtx_komodo.py-test_one_one_rewards_claim": "e53f221fda81469027e39e21877a81a8fafbffbece0a45aeda12aae8873b0464",
 "test_msg_signtx_peercoin.py::test_timestamp_included": "825b9bdf5238c5c6415a254a6bae4b2bd9df8fc5cb31f66f0c20145cb4e60bbb",
 "test_msg_signtx_segwit.py-test_attack_change_input_address": "5ae71202c062ef7942626a80a4ceeb8d8c4ea5065a97f0de6a97505e9cb82c2c",
-"test_msg_signtx_segwit.py-test_attack_mixed_inputs": "406596db037a13d5e26b3829a74b6efcde1d13159532a36fcd6b1b4bdcef3781",
+"test_msg_signtx_segwit.py-test_attack_mixed_inputs": "ed4cf8ff26ca1abb0adf20e1020dad7861b5e63ead2a1a9c0c5e9080d37f6f1c",
 "test_msg_signtx_segwit.py-test_send_multisig_1": "958a0741070e057dcb889b2000e5487d391bc513e4a5d86193a355261c5f361b",
 "test_msg_signtx_segwit.py-test_send_p2sh": "ca593e31e919b9e920289b13e4c70b9607f34b93d06ace69835e3d08ecf046c8",
 "test_msg_signtx_segwit.py-test_send_p2sh_change": "562c7ee5a2e264c9f93387dd165403dab32bb305a4c3a6143a902c4a4c9e5950",