From ae71735e62cc1f98e7f9523f9889ae0f97c3bf23 Mon Sep 17 00:00:00 2001 From: Andrew Kozlik Date: Thu, 9 Jul 2020 11:27:44 +0200 Subject: [PATCH] legacy/signing: Ask user to confirm custom nLockTime. --- legacy/firmware/CHANGELOG.md | 1 + legacy/firmware/layout2.c | 21 +++++++++++++++++ legacy/firmware/layout2.h | 2 ++ legacy/firmware/signing.c | 24 ++++++++++++++++++++ tests/device_tests/test_msg_signtx_komodo.py | 6 ++--- 5 files changed, 50 insertions(+), 4 deletions(-) diff --git a/legacy/firmware/CHANGELOG.md b/legacy/firmware/CHANGELOG.md index ec739ac3f..1d6c583ae 100644 --- a/legacy/firmware/CHANGELOG.md +++ b/legacy/firmware/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Added - XVG support. [#1165] +- Ask user to confirm custom nLockTime. ### Changed - Print inverted question mark for non-printable characters. diff --git a/legacy/firmware/layout2.c b/legacy/firmware/layout2.c index ed2556efa..9ce8f8c0f 100644 --- a/legacy/firmware/layout2.c +++ b/legacy/firmware/layout2.c @@ -37,6 +37,8 @@ #include "timer.h" #include "util.h" +#define LOCKTIME_TIMESTAMP_MIN_VALUE 500000000 + #if !BITCOIN_ONLY static const char *slip44_extras(uint32_t coin_type) { @@ -450,6 +452,25 @@ void layoutChangeCountOverThreshold(uint32_t change_count) { _("Continue?"), NULL); } +void layoutConfirmNondefaultLockTime(uint32_t lock_time, + bool lock_time_disabled) { + if (lock_time_disabled) { + layoutDialogSwipe(&bmp_icon_question, _("Cancel"), _("Confirm"), NULL, + _("Warning!"), _("Locktime is set but"), + _("will have no effect."), NULL, _("Continue?"), NULL); + + } else { + char str_locktime[11] = {0}; + snprintf(str_locktime, sizeof(str_locktime), "%" PRIu32, lock_time); + char *str_type = (lock_time < LOCKTIME_TIMESTAMP_MIN_VALUE) ? "blockheight:" + : "timestamp:"; + + layoutDialogSwipe(&bmp_icon_question, _("Cancel"), _("Confirm"), NULL, + _("Locktime for this"), _("transaction is set to"), + str_type, str_locktime, _("Continue?"), NULL); + } +} + void layoutSignMessage(const uint8_t *msg, uint32_t len) { const char **str = NULL; if (!is_valid_ascii(msg, len)) { diff --git a/legacy/firmware/layout2.h b/legacy/firmware/layout2.h index 4109adc1f..049b62033 100644 --- a/legacy/firmware/layout2.h +++ b/legacy/firmware/layout2.h @@ -53,6 +53,8 @@ void layoutConfirmTx(const CoinInfo *coin, uint64_t amount_out, uint64_t amount_fee); void layoutFeeOverThreshold(const CoinInfo *coin, uint64_t fee); void layoutChangeCountOverThreshold(uint32_t change_count); +void layoutConfirmNondefaultLockTime(uint32_t lock_time, + bool lock_time_disabled); void layoutSignMessage(const uint8_t *msg, uint32_t len); void layoutVerifyAddress(const CoinInfo *coin, const char *address); void layoutVerifyMessage(const uint8_t *msg, uint32_t len); diff --git a/legacy/firmware/signing.c b/legacy/firmware/signing.c index b3550f2e7..2b48979ed 100644 --- a/legacy/firmware/signing.c +++ b/legacy/firmware/signing.c @@ -75,6 +75,7 @@ static uint32_t lock_time = 0; static uint32_t expiry = 0; static uint32_t version_group_id = 0; static uint32_t timestamp = 0; +static uint32_t min_sequence = 0; #if !BITCOIN_ONLY static uint32_t branch_id = 0; #endif @@ -107,6 +108,10 @@ static uint32_t tx_weight; /* The maximum number of change-outputs allowed without user confirmation. */ #define MAX_SILENT_CHANGE_COUNT 2 +/* Setting nSequence to this value for every input in a transaction disables + nLockTime. */ +#define SEQUENCE_FINAL 0xffffffff + enum { SIGHASH_ALL = 1, SIGHASH_FORKID = 0x40, @@ -497,6 +502,7 @@ void signing_init(const SignTx *msg, const CoinInfo *_coin, memcpy(&root, _root, sizeof(HDNode)); version = msg->version; lock_time = msg->lock_time; + min_sequence = SEQUENCE_FINAL; if (!coin->overwintered) { if (msg->has_version_group_id) { @@ -782,12 +788,18 @@ static bool signing_check_input(const TxInputType *txinput) { } else { // single signature multisig_fp_mismatch = true; } + // remember the input bip32 path // change addresses must use the same bip32 path as all inputs extract_input_bip32_path(txinput); + + // remember the minimum nSequence value + if (txinput->sequence < min_sequence) min_sequence = txinput->sequence; + // compute segwit hashPrevouts & hashSequence tx_prevout_hash(&hasher_prevouts, txinput); tx_sequence_hash(&hasher_sequence, txinput); + #if !BITCOIN_ONLY if (coin->decred) { // serialize Decred prefix in Phase 1 @@ -800,6 +812,7 @@ static bool signing_check_input(const TxInputType *txinput) { tx_serialize_input_hash(&ti, txinput); } #endif + // hash prevout and script type to check it later (relevant for fee // computation) tx_prevout_hash(&hasher_check, txinput); @@ -914,6 +927,7 @@ static bool signing_confirm_tx(void) { return false; } } + uint64_t fee = 0; if (spending <= to_spend) { fee = to_spend - spending; @@ -939,6 +953,16 @@ static bool signing_confirm_tx(void) { } } + if (lock_time != 0) { + bool lock_time_disabled = (min_sequence == SEQUENCE_FINAL); + layoutConfirmNondefaultLockTime(lock_time, lock_time_disabled); + if (!protectButton(ButtonRequestType_ButtonRequest_SignTx, false)) { + fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); + signing_abort(); + return false; + } + } + // last confirmation layoutConfirmTx(coin, to_spend - change_spend, fee); if (!protectButton(ButtonRequestType_ButtonRequest_SignTx, false)) { diff --git a/tests/device_tests/test_msg_signtx_komodo.py b/tests/device_tests/test_msg_signtx_komodo.py index aa086d9c8..fe9698d62 100644 --- a/tests/device_tests/test_msg_signtx_komodo.py +++ b/tests/device_tests/test_msg_signtx_komodo.py @@ -60,7 +60,6 @@ class TestMsgSigntxKomodo: script_type=proto.OutputScriptType.PAYTOADDRESS, ) - trezor_core = client.features.model != "1" with client: client.set_expected_responses( [ @@ -71,7 +70,7 @@ class TestMsgSigntxKomodo: request_extra_data(0, 11, TXHASH_2807c), request_output(0), proto.ButtonRequest(code=B.ConfirmOutput), - (trezor_core, proto.ButtonRequest(code=B.SignTx)), + proto.ButtonRequest(code=B.SignTx), proto.ButtonRequest(code=B.SignTx), request_input(0), request_output(0), @@ -120,7 +119,6 @@ class TestMsgSigntxKomodo: script_type=proto.OutputScriptType.PAYTOADDRESS, ) - trezor_core = client.features.model != "1" with client: client.set_expected_responses( [ @@ -133,7 +131,7 @@ class TestMsgSigntxKomodo: proto.ButtonRequest(code=B.ConfirmOutput), request_output(1), proto.ButtonRequest(code=B.ConfirmOutput), - (trezor_core, proto.ButtonRequest(code=B.SignTx)), + proto.ButtonRequest(code=B.SignTx), proto.ButtonRequest(code=B.SignTx), request_input(0), request_output(0),