diff --git a/core/src/apps/wallet/sign_tx/helpers.py b/core/src/apps/wallet/sign_tx/helpers.py index d01f35b16..2964ccc47 100644 --- a/core/src/apps/wallet/sign_tx/helpers.py +++ b/core/src/apps/wallet/sign_tx/helpers.py @@ -171,8 +171,14 @@ def sanitize_sign_tx(tx: SignTx, coin: CoinInfo) -> SignTx: tx.inputs_count = tx.inputs_count if tx.inputs_count is not None else 0 tx.outputs_count = tx.outputs_count if tx.outputs_count is not None else 0 tx.coin_name = tx.coin_name if tx.coin_name is not None else "Bitcoin" - tx.expiry = tx.expiry if tx.expiry is not None else 0 - tx.timestamp = tx.timestamp if tx.timestamp is not None else 0 + if coin.decred or coin.overwintered: + tx.expiry = tx.expiry if tx.expiry is not None else 0 + elif tx.expiry: + raise SigningError(FailureType.DataError, "Expiry not enabled on this coin.") + if coin.timestamp: + tx.timestamp = tx.timestamp if tx.timestamp is not None else 0 + elif tx.timestamp: + raise SigningError(FailureType.DataError, "Timestamp not enabled on this coin.") return tx @@ -181,9 +187,20 @@ def sanitize_tx_meta(tx: TransactionType, coin: CoinInfo) -> TransactionType: tx.lock_time = tx.lock_time if tx.lock_time is not None else 0 tx.inputs_cnt = tx.inputs_cnt if tx.inputs_cnt is not None else 0 tx.outputs_cnt = tx.outputs_cnt if tx.outputs_cnt is not None else 0 - tx.extra_data_len = tx.extra_data_len if tx.extra_data_len is not None else 0 - tx.expiry = tx.expiry if tx.expiry is not None else 0 - tx.timestamp = tx.timestamp if tx.timestamp is not None else 0 + if coin.extra_data: + tx.extra_data_len = tx.extra_data_len if tx.extra_data_len is not None else 0 + elif tx.extra_data_len: + raise SigningError( + FailureType.DataError, "Extra data not enabled on this coin." + ) + if coin.decred or coin.overwintered: + tx.expiry = tx.expiry if tx.expiry is not None else 0 + elif tx.expiry: + raise SigningError(FailureType.DataError, "Expiry not enabled on this coin.") + if coin.timestamp: + tx.timestamp = tx.timestamp if tx.timestamp is not None else 0 + elif tx.timestamp: + raise SigningError(FailureType.DataError, "Timestamp not enabled on this coin.") return tx diff --git a/core/src/apps/wallet/sign_tx/signing.py b/core/src/apps/wallet/sign_tx/signing.py index dee13b1b7..cbdb276cb 100644 --- a/core/src/apps/wallet/sign_tx/signing.py +++ b/core/src/apps/wallet/sign_tx/signing.py @@ -413,7 +413,7 @@ async def sign_tx(tx: SignTx, keychain: seed.Keychain): h_second = utils.HashWriter(sha256()) writers.write_uint32(h_sign, tx.version) # nVersion - if tx.timestamp: + if not utils.BITCOIN_ONLY and coin.timestamp: writers.write_uint32(h_sign, tx.timestamp) writers.write_varint(h_sign, tx.inputs_count) @@ -615,7 +615,7 @@ async def get_prevtx_output_value( writers.write_uint32(txh, tx.version | decred.DECRED_SERIALIZE_NO_WITNESS) else: writers.write_uint32(txh, tx.version) # nVersion - if tx.timestamp: + if not utils.BITCOIN_ONLY and coin.timestamp: writers.write_uint32(txh, tx.timestamp) writers.write_varint(txh, tx.inputs_cnt) @@ -652,12 +652,13 @@ async def get_prevtx_output_value( if not utils.BITCOIN_ONLY and (coin.overwintered or coin.decred): writers.write_uint32(txh, tx.expiry) - ofs = 0 - while ofs < tx.extra_data_len: - size = min(1024, tx.extra_data_len - ofs) - data = await helpers.request_tx_extra_data(tx_req, ofs, size, prev_hash) - writers.write_bytes(txh, data) - ofs += len(data) + if not utils.BITCOIN_ONLY and coin.extra_data: + ofs = 0 + while ofs < tx.extra_data_len: + size = min(1024, tx.extra_data_len - ofs) + data = await helpers.request_tx_extra_data(tx_req, ofs, size, prev_hash) + writers.write_bytes(txh, data) + ofs += len(data) if ( writers.get_tx_hash(txh, double=coin.sign_hash_double, reverse=True) @@ -690,7 +691,7 @@ def get_tx_header(coin: coininfo.CoinInfo, tx: SignTx, segwit: bool): writers.write_uint32(w_txi, tx.version_group_id) # nVersionGroupId else: writers.write_uint32(w_txi, tx.version) # nVersion - if tx.timestamp: + if not utils.BITCOIN_ONLY and coin.timestamp: writers.write_uint32(w_txi, tx.timestamp) if segwit: writers.write_varint(w_txi, 0x00) # segwit witness marker diff --git a/legacy/firmware/fsm_msg_coin.h b/legacy/firmware/fsm_msg_coin.h index 65d621eab..24ccee89d 100644 --- a/legacy/firmware/fsm_msg_coin.h +++ b/legacy/firmware/fsm_msg_coin.h @@ -102,6 +102,12 @@ void fsm_msgSignTx(const SignTx *msg) { const CoinInfo *coin = fsm_getCoin(msg->has_coin_name, msg->coin_name); if (!coin) return; + + CHECK_PARAM((coin->decred || coin->overwintered) || !msg->has_expiry, + _("Expiry not enabled on this coin.")) + CHECK_PARAM(coin->timestamp || !msg->has_timestamp, + _("Timestamp not enabled on this coin.")) + const HDNode *node = fsm_getDerivedNode(coin->curve_name, NULL, 0, NULL); if (!node) return; diff --git a/legacy/firmware/signing.c b/legacy/firmware/signing.c index 4cc9944f6..4205f22b5 100644 --- a/legacy/firmware/signing.c +++ b/legacy/firmware/signing.c @@ -41,7 +41,9 @@ enum { STAGE_REQUEST_2_PREV_META, STAGE_REQUEST_2_PREV_INPUT, STAGE_REQUEST_2_PREV_OUTPUT, +#if !BITCOIN_ONLY STAGE_REQUEST_2_PREV_EXTRADATA, +#endif STAGE_REQUEST_3_OUTPUT, STAGE_REQUEST_4_INPUT, STAGE_REQUEST_4_OUTPUT, @@ -268,6 +270,8 @@ void send_req_2_prev_output(void) { msg_write(MessageType_MessageType_TxRequest, &resp); } +#if !BITCOIN_ONLY + void send_req_2_prev_extradata(uint32_t chunk_offset, uint32_t chunk_len) { signing_stage = STAGE_REQUEST_2_PREV_EXTRADATA; resp.has_request_type = true; @@ -284,6 +288,8 @@ void send_req_2_prev_extradata(uint32_t chunk_offset, uint32_t chunk_len) { msg_write(MessageType_MessageType_TxRequest, &resp); } +#endif + void send_req_3_output(void) { signing_stage = STAGE_REQUEST_3_OUTPUT; resp.has_request_type = true; @@ -483,21 +489,26 @@ void signing_init(const SignTx *msg, const CoinInfo *_coin, memcpy(&root, _root, sizeof(HDNode)); version = msg->version; lock_time = msg->lock_time; - expiry = msg->expiry; #if !BITCOIN_ONLY - version_group_id = msg->version_group_id; - timestamp = msg->timestamp; - branch_id = msg->branch_id; - // set default values for Zcash if branch_id is unset - if (coin->overwintered && (branch_id == 0)) { - switch (version) { - case 3: - branch_id = 0x5BA81B19; // Overwinter - break; - case 4: - branch_id = 0x76B809BB; // Sapling - break; + expiry = (coin->decred || coin->overwintered) ? msg->expiry : 0; + timestamp = coin->timestamp ? msg->timestamp : 0; + if (coin->overwintered) { + version_group_id = msg->version_group_id; + branch_id = msg->branch_id; + if (branch_id == 0) { + // set default values for Zcash if branch_id is unset + switch (version) { + case 3: + branch_id = 0x5BA81B19; // Overwinter + break; + case 4: + branch_id = 0x76B809BB; // Sapling + break; + } } + } else { + version_group_id = 0; + branch_id = 0; } #endif @@ -1221,6 +1232,24 @@ void signing_txack(TransactionType *tx) { signing_abort(); return; } + if (!coin->extra_data && tx->extra_data_len > 0) { + fsm_sendFailure(FailureType_Failure_DataError, + _("Extra data not enabled on this coin.")); + signing_abort(); + return; + } + if (!coin->decred && !coin->overwintered && tx->has_expiry) { + fsm_sendFailure(FailureType_Failure_DataError, + _("Expiry not enabled on this coin.")); + signing_abort(); + return; + } + if (!coin->timestamp && tx->has_timestamp) { + fsm_sendFailure(FailureType_Failure_DataError, + _("Timestamp not enabled on this coin.")); + signing_abort(); + return; + } if (tx->inputs_cnt + tx->outputs_cnt < tx->inputs_cnt) { fsm_sendFailure(FailureType_Failure_DataError, _("Value overflow")); signing_abort(); @@ -1292,14 +1321,17 @@ void signing_txack(TransactionType *tx) { /* Check prevtx of next input */ idx2++; send_req_2_prev_output(); - } else if (tp.extra_data_len > 0) { // has extra data +#if !BITCOIN_ONLY + } else if (coin->extra_data && tp.extra_data_len > 0) { // has extra data send_req_2_prev_extradata(0, MIN(1024, tp.extra_data_len)); return; +#endif } else { /* prevtx is done */ signing_check_prevtx_hash(); } return; +#if !BITCOIN_ONLY case STAGE_REQUEST_2_PREV_EXTRADATA: if (!tx_serialize_extra_data_hash(&tp, tx->extra_data.bytes, tx->extra_data.size)) { @@ -1317,6 +1349,7 @@ void signing_txack(TransactionType *tx) { signing_check_prevtx_hash(); } return; +#endif case STAGE_REQUEST_3_OUTPUT: if (!signing_check_output(&tx->outputs[0])) { return; diff --git a/legacy/firmware/transaction.c b/legacy/firmware/transaction.c index 91e2d6aff..450a165d1 100644 --- a/legacy/firmware/transaction.c +++ b/legacy/firmware/transaction.c @@ -504,17 +504,22 @@ uint32_t tx_serialize_script(uint32_t size, const uint8_t *data, uint8_t *out) { uint32_t tx_serialize_header(TxStruct *tx, uint8_t *out) { int r = 4; +#if !BITCOIN_ONLY if (tx->overwintered) { uint32_t ver = tx->version | TX_OVERWINTERED; memcpy(out, &ver, 4); memcpy(out + 4, &(tx->version_group_id), 4); r += 4; - } else { + } else +#endif + { memcpy(out, &(tx->version), 4); +#if !BITCOIN_ONLY if (tx->timestamp) { memcpy(out + r, &(tx->timestamp), 4); r += 4; } +#endif if (tx->is_segwit) { memcpy(out + r, segwit_header, 2); r += 2; @@ -525,16 +530,21 @@ uint32_t tx_serialize_header(TxStruct *tx, uint8_t *out) { uint32_t tx_serialize_header_hash(TxStruct *tx) { int r = 4; +#if !BITCOIN_ONLY if (tx->overwintered) { uint32_t ver = tx->version | TX_OVERWINTERED; hasher_Update(&(tx->hasher), (const uint8_t *)&ver, 4); hasher_Update(&(tx->hasher), (const uint8_t *)&(tx->version_group_id), 4); r += 4; - } else { + } else +#endif + { hasher_Update(&(tx->hasher), (const uint8_t *)&(tx->version), 4); +#if !BITCOIN_ONLY if (tx->timestamp) { hasher_Update(&(tx->hasher), (const uint8_t *)&(tx->timestamp), 4); } +#endif if (tx->is_segwit) { hasher_Update(&(tx->hasher), segwit_header, 2); r += 2; @@ -559,10 +569,13 @@ uint32_t tx_serialize_input(TxStruct *tx, const TxInputType *input, r += 32; memcpy(out + r, &input->prev_index, 4); r += 4; +#if !BITCOIN_ONLY if (tx->is_decred) { uint8_t tree = input->decred_tree & 0xFF; out[r++] = tree; - } else { + } else +#endif + { r += tx_serialize_script(input->script_sig.size, input->script_sig.bytes, out + r); } @@ -585,11 +598,14 @@ uint32_t tx_serialize_input_hash(TxStruct *tx, const TxInputType *input) { r += tx_serialize_header_hash(tx); } r += tx_prevout_hash(&(tx->hasher), input); +#if !BITCOIN_ONLY if (tx->is_decred) { uint8_t tree = input->decred_tree & 0xFF; hasher_Update(&(tx->hasher), (const uint8_t *)&(tree), 1); r++; - } else { + } else +#endif + { r += tx_script_hash(&(tx->hasher), input->script_sig.size, input->script_sig.bytes); } @@ -601,6 +617,7 @@ uint32_t tx_serialize_input_hash(TxStruct *tx, const TxInputType *input) { return r; } +#if !BITCOIN_ONLY uint32_t tx_serialize_decred_witness(TxStruct *tx, const TxInputType *input, uint8_t *out) { static const uint64_t amount = 0; @@ -652,6 +669,7 @@ uint32_t tx_serialize_decred_witness_hash(TxStruct *tx, return r; } +#endif uint32_t tx_serialize_middle(TxStruct *tx, uint8_t *out) { return ser_length(tx->outputs_len, out); @@ -663,6 +681,7 @@ uint32_t tx_serialize_middle_hash(TxStruct *tx) { uint32_t tx_serialize_footer(TxStruct *tx, uint8_t *out) { memcpy(out, &(tx->lock_time), 4); +#if !BITCOIN_ONLY if (tx->overwintered) { if (tx->version == 3) { memcpy(out + 4, &(tx->expiry), 4); @@ -681,11 +700,13 @@ uint32_t tx_serialize_footer(TxStruct *tx, uint8_t *out) { memcpy(out + 4, &(tx->expiry), 4); return 8; } +#endif return 4; } uint32_t tx_serialize_footer_hash(TxStruct *tx) { hasher_Update(&(tx->hasher), (const uint8_t *)&(tx->lock_time), 4); +#if !BITCOIN_ONLY if (tx->overwintered) { hasher_Update(&(tx->hasher), (const uint8_t *)&(tx->expiry), 4); hasher_Update(&(tx->hasher), (const uint8_t *)"\x00", 1); // nJoinSplit @@ -695,6 +716,7 @@ uint32_t tx_serialize_footer_hash(TxStruct *tx) { hasher_Update(&(tx->hasher), (const uint8_t *)&(tx->expiry), 4); return 8; } +#endif return 4; } @@ -714,11 +736,13 @@ uint32_t tx_serialize_output(TxStruct *tx, const TxOutputBinType *output, } memcpy(out + r, &output->amount, 8); r += 8; +#if !BITCOIN_ONLY if (tx->is_decred) { uint16_t script_version = output->decred_script_version & 0xFFFF; memcpy(out + r, &script_version, 2); r += 2; } +#endif r += tx_serialize_script(output->script_pubkey.size, output->script_pubkey.bytes, out + r); tx->have_outputs++; @@ -751,6 +775,7 @@ uint32_t tx_serialize_output_hash(TxStruct *tx, const TxOutputBinType *output) { return r; } +#if !BITCOIN_ONLY uint32_t tx_serialize_extra_data_hash(TxStruct *tx, const uint8_t *data, uint32_t datalen) { if (tx->have_inputs < tx->inputs_len) { @@ -770,6 +795,7 @@ uint32_t tx_serialize_extra_data_hash(TxStruct *tx, const uint8_t *data, tx->size += datalen; return datalen; } +#endif void tx_init(TxStruct *tx, uint32_t inputs_len, uint32_t outputs_len, uint32_t version, uint32_t lock_time, uint32_t expiry, @@ -903,6 +929,7 @@ uint32_t tx_output_weight(const CoinInfo *coin, const TxOutputType *txoutput) { return 4 * (size + output_script_size); } +#if !BITCOIN_ONLY uint32_t tx_decred_witness_weight(const TxInputType *txinput) { uint32_t input_script_size = tx_input_script_size(txinput); uint32_t size = TXSIZE_DECRED_WITNESS + ser_length_size(input_script_size) + @@ -910,3 +937,4 @@ uint32_t tx_decred_witness_weight(const TxInputType *txinput) { return 4 * size; } +#endif diff --git a/tests/device_tests/test_msg_signtx.py b/tests/device_tests/test_msg_signtx.py index d823f0bb3..b71730104 100644 --- a/tests/device_tests/test_msg_signtx.py +++ b/tests/device_tests/test_msg_signtx.py @@ -893,3 +893,44 @@ class TestMsgSigntx: assert e.value.failure.message.endswith( "Not enough outputs in previous transaction." ) + + @pytest.mark.parametrize( + "field, value", + (("extra_data", b"hello world"), ("expiry", 9), ("timestamp", 42)), + ) + @pytest.mark.skip_ui + def test_prevtx_forbidden_fields(self, client, field, value): + cache = tx_cache("Bitcoin") + inp0 = proto.TxInputType(address_n=[0], prev_hash=TXHASH_157041, prev_index=0) + out1 = proto.TxOutputType( + address="1MJ2tj2ThBE62zXbBYA5ZaN3fdve5CPAz1", + amount=1000, + script_type=proto.OutputScriptType.PAYTOADDRESS, + ) + + prev_tx = cache[TXHASH_157041] + setattr(prev_tx, field, value) + with pytest.raises(TrezorFailure) as e: + btc.sign_tx( + client, "Bitcoin", [inp0], [out1], prev_txes={TXHASH_157041: prev_tx} + ) + name = field[0].upper() + field[1:].replace("_", " ") + assert e.value.failure.message.endswith(name + " not enabled on this coin.") + + @pytest.mark.parametrize("field, value", (("expiry", 9), ("timestamp", 42))) + @pytest.mark.skip_ui + def test_signtx_forbidden_fields(self, client, field, value): + cache = tx_cache("Bitcoin") + inp0 = proto.TxInputType(address_n=[0], prev_hash=TXHASH_157041, prev_index=0) + out1 = proto.TxOutputType( + address="1MJ2tj2ThBE62zXbBYA5ZaN3fdve5CPAz1", + amount=1000, + script_type=proto.OutputScriptType.PAYTOADDRESS, + ) + + details = proto.SignTx() + setattr(details, field, value) + with pytest.raises(TrezorFailure) as e: + btc.sign_tx(client, "Bitcoin", [inp0], [out1], details, prev_txes=cache) + name = field[0].upper() + field[1:].replace("_", " ") + assert e.value.failure.message.endswith(name + " not enabled on this coin.")