diff --git a/core/CHANGELOG.md b/core/CHANGELOG.md index 954816a028..e2424ee156 100644 --- a/core/CHANGELOG.md +++ b/core/CHANGELOG.md @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Running the frozen version of the emulator doesn't need arguments. [#1115] - CoinJoin preauthorization and siging flow. [#1053] - XVG support. [#1165] +- Hard limit on transaction fees. Can be disabled using `safety-checks`. [#1087] ### Changed - Print inverted question mark for non-printable characters. @@ -257,6 +258,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 diff --git a/core/src/apps/bitcoin/sign_tx/approvers.py b/core/src/apps/bitcoin/sign_tx/approvers.py index 54808ef279..eacc2fe055 100644 --- a/core/src/apps/bitcoin/sign_tx/approvers.py +++ b/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 @@ class BasicApprover(Approver): 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) diff --git a/tests/device_tests/test_msg_signtx.py b/tests/device_tests/test_msg_signtx.py index 615103d273..8814c4528d 100644 --- a/tests/device_tests/test_msg_signtx.py +++ b/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 @@ class TestMsgSigntx: == "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 @@ class TestMsgSigntx: out1 = messages.TxOutputType( address="mfiGQVPcRcaEvQPYDErR34DcCovtxYvUUV", - amount=1000000000 - 500000000 - 100000000, + amount=1000000000 - 500000000 - 8000000, script_type=messages.OutputScriptType.PAYTOADDRESS, ) @@ -221,7 +221,7 @@ class TestMsgSigntx: assert ( tx_hash(serialized_tx).hex() - == "c669527e0d80dc645925f6965e1622e71fa5ca51e284442c4620c1ade7a76c63" + == "54fd5e9b65b8acc10144c1e78ea9720df7606d7d4a543e4c547ecd45b2ae226b" ) def test_one_two_fee(self, client): @@ -577,7 +577,7 @@ class TestMsgSigntx: == "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 @@ class TestMsgSigntx: == "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 diff --git a/tests/device_tests/test_msg_signtx_prevhash.py b/tests/device_tests/test_msg_signtx_prevhash.py index ecbe63abcf..13a0b29dea 100644 --- a/tests/device_tests/test_msg_signtx_prevhash.py +++ b/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}) diff --git a/tests/device_tests/test_msg_signtx_segwit.py b/tests/device_tests/test_msg_signtx_segwit.py index db757a1d26..fe7c7fa274 100644 --- a/tests/device_tests/test_msg_signtx_segwit.py +++ b/tests/device_tests/test_msg_signtx_segwit.py @@ -362,7 +362,7 @@ class TestMsgSigntxSegwit: ) out1 = proto.TxOutputType( address="mhRx1CeVfaayqRwq5zgRQmD7W5aWBfD5mC", - amount=30998000, + amount=31000000 + TRUE_AMOUNT - 3456789, script_type=proto.OutputScriptType.PAYTOADDRESS, ) diff --git a/tests/ui_tests/fixtures.json b/tests/ui_tests/fixtures.json index c16bcd487b..ee71d84fdc 100644 --- a/tests/ui_tests/fixtures.json +++ b/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",