From e108ba5bde69b8b493d5fe7ff8377e2767accba8 Mon Sep 17 00:00:00 2001 From: Andrew Kozlik Date: Mon, 7 Mar 2022 20:19:03 +0100 Subject: [PATCH] fix(tests): Tests fail earlier due to scriptPubKey check. --- tests/device_tests/bitcoin/test_multisig.py | 24 +------ .../bitcoin/test_signtx_invalid_path.py | 65 ++++--------------- .../bitcoin/test_signtx_segwit.py | 27 ++------ tests/ui_tests/fixtures.json | 12 ++-- 4 files changed, 25 insertions(+), 103 deletions(-) diff --git a/tests/device_tests/bitcoin/test_multisig.py b/tests/device_tests/bitcoin/test_multisig.py index 13d9be6bd..d309885ce 100644 --- a/tests/device_tests/bitcoin/test_multisig.py +++ b/tests/device_tests/bitcoin/test_multisig.py @@ -317,26 +317,7 @@ def test_attack_change_input(client: Client): with client: client.set_filter(messages.TxAck, attack_processor) - client.set_expected_responses( - [ - request_input(0), - request_output(0), - messages.ButtonRequest(code=B.ConfirmOutput), - request_output(1), - messages.ButtonRequest(code=B.SignTx), - request_input(0), - request_meta(TXHASH_509e08), - request_input(0, TXHASH_509e08), - request_output(0, TXHASH_509e08), - request_input(0), - request_output(0), - request_output(1), - request_input(0), - messages.Failure(code=messages.FailureType.ProcessError), - ] - ) - - with pytest.raises(TrezorFailure) as exc: + with pytest.raises(TrezorFailure): btc.sign_tx( client, "Testnet", @@ -344,6 +325,3 @@ def test_attack_change_input(client: Client): [output_payee, output_change], prev_txes=TX_API_TESTNET, ) - - assert exc.value.code == messages.FailureType.ProcessError - assert exc.value.message.endswith("Transaction has changed during signing") diff --git a/tests/device_tests/bitcoin/test_signtx_invalid_path.py b/tests/device_tests/bitcoin/test_signtx_invalid_path.py index 32a717b7a..cb8020133 100644 --- a/tests/device_tests/bitcoin/test_signtx_invalid_path.py +++ b/tests/device_tests/bitcoin/test_signtx_invalid_path.py @@ -21,13 +21,7 @@ from trezorlib.debuglink import TrezorClientDebugLink as Client from trezorlib.exceptions import TrezorFailure from trezorlib.tools import H_, parse_path -from .signtx import ( - forge_prevtx, - request_finished, - request_input, - request_meta, - request_output, -) +from .signtx import forge_prevtx, request_input B = messages.ButtonRequestType @@ -113,27 +107,27 @@ def test_attack_path_segwit(client: Client): # avoid the path warning dialog, but in step6_sign_segwit_inputs() uses Bitcoin paths # to get a valid signature. + device.apply_settings( + client, safety_checks=messages.SafetyCheckLevel.PromptTemporarily + ) + # Generate keys address_a = btc.get_address( client, "Testnet", - parse_path("m/84h/1h/0h/0/0"), + parse_path("m/84h/0h/0h/0/0"), script_type=messages.InputScriptType.SPENDWITNESS, ) address_b = btc.get_address( client, "Testnet", - parse_path("m/84h/1h/1h/0/1"), + parse_path("m/84h/0h/1h/0/1"), script_type=messages.InputScriptType.SPENDWITNESS, ) prev_hash, prev_tx = forge_prevtx( [(address_a, 9_426), (address_b, 7_086)], network="testnet" ) - device.apply_settings( - client, safety_checks=messages.SafetyCheckLevel.PromptTemporarily - ) - inp1 = messages.TxInputType( # The actual input that the attacker wants to get signed. address_n=parse_path("m/84h/0h/0h/0/0"), @@ -174,47 +168,10 @@ def test_attack_path_segwit(client: Client): with client: client.set_filter(messages.TxAck, attack_processor) - client.set_expected_responses( - [ - # Step: process inputs - request_input(0), - # Attacker bypasses warning about non-standard path. - request_input(1), - # Attacker bypasses warning about non-standard path. - # Step: approve outputs - request_output(0), - messages.ButtonRequest(code=B.ConfirmOutput), - messages.ButtonRequest(code=B.SignTx), - # Step: verify inputs - request_input(0), - request_meta(prev_hash), - request_input(0, prev_hash), - request_output(0, prev_hash), - request_output(1, prev_hash), - request_input(1), - request_meta(prev_hash), - request_input(0, prev_hash), - request_output(0, prev_hash), - request_output(1, prev_hash), - # Step: serialize inputs - request_input(0), - request_input(1), - # Step: serialize outputs - request_output(0), - # Step: sign segwit inputs - request_input(0), - # Trezor must warn about non-standard path before signing. - messages.ButtonRequest(code=B.UnknownDerivationPath), - request_input(1), - # Trezor must warn about non-standard path before signing. - messages.ButtonRequest(code=B.UnknownDerivationPath), - request_finished(), - ] - ) - - btc.sign_tx( - client, "Testnet", [inp1, inp2], [out1], prev_txes={prev_hash: prev_tx} - ) + with pytest.raises(TrezorFailure): + btc.sign_tx( + client, "Testnet", [inp1, inp2], [out1], prev_txes={prev_hash: prev_tx} + ) @pytest.mark.skip_t1(reason="T1 only prevents using paths known to be altcoins") diff --git a/tests/device_tests/bitcoin/test_signtx_segwit.py b/tests/device_tests/bitcoin/test_signtx_segwit.py index bc7c8dcbc..0065cebbf 100644 --- a/tests/device_tests/bitcoin/test_signtx_segwit.py +++ b/tests/device_tests/bitcoin/test_signtx_segwit.py @@ -266,6 +266,10 @@ def test_send_multisig_1(client: Client): def test_attack_change_input_address(client: Client): + # Simulates an attack where the user is coerced into unknowingly + # transferring funds from one account to another one of their accounts, + # potentially resulting in privacy issues. + inp1 = messages.TxInputType( address_n=parse_path("m/49h/1h/0h/1/0"), # 2N1LGaGg836mqSQqiuUBLfcyGBhyZbremDX @@ -285,12 +289,13 @@ def test_attack_change_input_address(client: Client): amount=123_456_789 - 11_000 - 12_300_000, ) - # Test if the transaction can be signed normally + # Test if the transaction can be signed normally. with client: client.set_expected_responses( [ request_input(0), request_output(0), + # The user is required to confirm transfer to another account. messages.ButtonRequest(code=B.ConfirmOutput), request_output(1), messages.ButtonRequest(code=B.ConfirmOutput), @@ -330,28 +335,10 @@ def test_attack_change_input_address(client: Client): # Now run the attack, must trigger the exception with client: client.set_filter(messages.TxAck, attack_processor) - client.set_expected_responses( - [ - request_input(0), - request_output(0), - messages.ButtonRequest(code=B.ConfirmOutput), - request_output(1), - messages.ButtonRequest(code=B.SignTx), - request_input(0), - request_meta(TXHASH_20912f), - request_input(0, TXHASH_20912f), - request_output(0, TXHASH_20912f), - request_output(1, TXHASH_20912f), - request_input(0), - messages.Failure(code=messages.FailureType.ProcessError), - ] - ) - with pytest.raises(TrezorFailure) as exc: + with pytest.raises(TrezorFailure): btc.sign_tx( client, "Testnet", [inp1], [out1, out2], prev_txes=TX_API_TESTNET ) - assert exc.value.code == messages.FailureType.ProcessError - assert exc.value.message.endswith("Transaction has changed during signing") def test_attack_mixed_inputs(client: Client): diff --git a/tests/ui_tests/fixtures.json b/tests/ui_tests/fixtures.json index da79db499..66f934ecf 100644 --- a/tests/ui_tests/fixtures.json +++ b/tests/ui_tests/fixtures.json @@ -249,7 +249,7 @@ "T1_bitcoin-test_signtx_amount_unit.py::test_signtx[AmountUnit.MILLIBITCOIN]": "8081910bd937c704fb65226dc85e8fc58a7331102aa036a5a97652638a0bde20", "T1_bitcoin-test_signtx_amount_unit.py::test_signtx[AmountUnit.SATOSHI]": "a565b50e63776aad284c42c71bf53e5844dff12f159cbd00679b97d2fb74fd20", "T1_bitcoin-test_signtx_amount_unit.py::test_signtx[None]": "367fd3c75f30f7224435e3309562d210d9ac6809ce2fae392f7fe75070fda094", -"T1_bitcoin-test_signtx_invalid_path.py::test_attack_path_segwit": "01e5defbadae9662efd5dc3a042264c78ea03176f9377ca627dd49301063b820", +"T1_bitcoin-test_signtx_invalid_path.py::test_attack_path_segwit": "473636ae4c43d8a349db09187b74eb4c1aa2b7fe02742d5fa928cdbc2a9e4cfd", "T1_bitcoin-test_signtx_invalid_path.py::test_invalid_path_fail": "75e45c0b6039244afae5cb138aeb4eec2c01e71b91a3ce0d73797ca3b04ca94a", "T1_bitcoin-test_signtx_invalid_path.py::test_invalid_path_pass_forkid": "9c35bfcc194afff453802147f5b4a2d033492e564835be649c3293a62857de59", "T1_bitcoin-test_signtx_invalid_path.py::test_invalid_path_prompt": "57b20ea95e26ee7f675cbb177b9c781389ef36a02f2815f7939ab83c2d0f8f36", @@ -762,7 +762,7 @@ "TT_bitcoin-test_komodo.py::test_one_one_rewards_claim": "b3c056df25d639927faaf16dc18c281c1a36b790ea4e77f954f681fb27d3fa1a", "TT_bitcoin-test_multisig.py::test_15_of_15": "230fd7974beb81c3824cb89f373336be80bfc62954220e63f0321e13d57ae8db", "TT_bitcoin-test_multisig.py::test_2_of_3": "49fea8705e96436830613412daad4f87f5b940dc4674aec23e0fd013a81afc68", -"TT_bitcoin-test_multisig.py::test_attack_change_input": "4c689fde3fcfa29d34d0889e995f0a5b7c784d6eb5d814dd51aa0624a179e2ac", +"TT_bitcoin-test_multisig.py::test_attack_change_input": "0455575766d47245b0943a14e46a0d31a3e8fba4f0ccce5bb76024897140ecd4", "TT_bitcoin-test_multisig.py::test_missing_pubkey": "1c100ce4b7c1e47e72428f390de0846c1ff933e9f07894872644a369a9422738", "TT_bitcoin-test_multisig_change.py::test_external_external": "e8a36ea9a3abeca8d7ac862f384c6ad49929b00e8c291ef49807d6b0c7798c5e", "TT_bitcoin-test_multisig_change.py::test_external_internal": "f6397edb70e13528fb2821b9097f72300bc76460deb32649b5bff9f76707d5b3", @@ -794,8 +794,8 @@ "TT_bitcoin-test_nonstandard_paths.py::test_signmessage[m-3h-100h-4-255-script_types1]": "4f73135d2ec9add695e0a22d855816558b4ba9329a2828f9c9930be6245bdc2d", "TT_bitcoin-test_nonstandard_paths.py::test_signmessage[m-4-255-script_types0]": "0988cc8bdc5879744bd33190fddc5b5aa137fdd7214abb003c8000a871d98f14", "TT_bitcoin-test_nonstandard_paths.py::test_signmessage[m-49-0-63-0-255-script_types4]": "540df94c73a4eed8fe88cdb475e2b31df752dca9e47b102792c01064ee432752", -"TT_bitcoin-test_nonstandard_paths.py::test_signtx[m-1195487518-6-255-script_types3]": "3fb1ec777c4c1a4e320740d050444077e118a0fbcfec96cb7e5ead203dfe01a2", -"TT_bitcoin-test_nonstandard_paths.py::test_signtx[m-1195487518-script_types2]": "e83d90183a5899d8881271e27ce030ec252df9c4a32ca4097cad811431553c37", +"TT_bitcoin-test_nonstandard_paths.py::test_signtx[m-1195487518-6-255-script_types3]": "37cfe119620536464ae42b3fbcae7b89d9272ad904da2bd8e8ae47b1024b4007", +"TT_bitcoin-test_nonstandard_paths.py::test_signtx[m-1195487518-script_types2]": "27a03a5be542d1f5f76a839e65daec766c1d7de8ae4637404ffcfea8267ea0ec", "TT_bitcoin-test_nonstandard_paths.py::test_signtx[m-3h-100h-4-255-script_types1]": "efbe785820901471b0e55f9fd743c84a29fe719c2e1c8e6b2f87b0a20ce43cb2", "TT_bitcoin-test_nonstandard_paths.py::test_signtx[m-4-255-script_types0]": "efbe785820901471b0e55f9fd743c84a29fe719c2e1c8e6b2f87b0a20ce43cb2", "TT_bitcoin-test_nonstandard_paths.py::test_signtx[m-49-0-63-0-255-script_types4]": "4392475bb51d2dd9316036ed268ee84bafb6f3f7b0d2e1ab6be69a63775d5f66", @@ -896,10 +896,10 @@ "TT_bitcoin-test_signtx_external.py::test_p2wpkh_with_false_proof": "ca3bdc82d0ddd668d50635ddbc91019095311e0c165094a89b9ae6eda53abdd6", "TT_bitcoin-test_signtx_external.py::test_p2wpkh_with_proof": "f5f2f9bc3c50908ce41046096586387b0db86796c54cb70216e4fedbb68995c6", "TT_bitcoin-test_signtx_external.py::test_p2wsh_external_presigned": "cd1a603df8ce086697c290aa63b9d922f8360ee5b34ff0dbe5eec4f9c4dd8636", -"TT_bitcoin-test_signtx_invalid_path.py::test_attack_path_segwit": "3feaa01d47aa9757e9d74f668927fd1445493c367adff94215405eb8e0a2749b", +"TT_bitcoin-test_signtx_invalid_path.py::test_attack_path_segwit": "3274688aba04218c47edcdd07c6164149e290ee414707fa603c7b4d7eda310f4", "TT_bitcoin-test_signtx_invalid_path.py::test_invalid_path_fail": "1c100ce4b7c1e47e72428f390de0846c1ff933e9f07894872644a369a9422738", "TT_bitcoin-test_signtx_invalid_path.py::test_invalid_path_fail_asap": "1c100ce4b7c1e47e72428f390de0846c1ff933e9f07894872644a369a9422738", -"TT_bitcoin-test_signtx_invalid_path.py::test_invalid_path_pass_forkid": "ef98eb752ec5fa948c952def7599f57a2bc5240b2d6b1eec0e02cc9be5c3040f", +"TT_bitcoin-test_signtx_invalid_path.py::test_invalid_path_pass_forkid": "85d3c2f3c85e1bcf774f3067d7eb32396c444f351ad15e68a328f87cf6bdb338", "TT_bitcoin-test_signtx_invalid_path.py::test_invalid_path_prompt": "12e137210397357ed754af0f4618ef03312b3e884930f55727d1b034f969bfd5", "TT_bitcoin-test_signtx_mixed_inputs.py::test_non_segwit_segwit_inputs": "77ee4c0c509ca4153fb78cfec0a02efeb738b20d6c9408933b41669e9e66eb55", "TT_bitcoin-test_signtx_mixed_inputs.py::test_non_segwit_segwit_non_segwit_inputs": "380823ef5cec4ba653e1e2fac2ecd77377654e445950aef4f2fb38da6fc883e1",