all: ensure expiry, timestamp and extra_data are blocked as appropriate

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
+    if coin.decred or coin.overwintered:
-    tx.timestamp = tx.timestamp if tx.timestamp is not None else 0
+        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
+    if coin.extra_data:
-    tx.expiry = tx.expiry if tx.expiry is not None else 0
+        tx.extra_data_len = tx.extra_data_len if tx.extra_data_len is not None else 0
-    tx.timestamp = tx.timestamp if tx.timestamp 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
 
 

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
+    if not utils.BITCOIN_ONLY and coin.extra_data:
-    while ofs < tx.extra_data_len:
+        ofs = 0
-        size = min(1024, tx.extra_data_len - ofs)
+        while ofs < tx.extra_data_len:
-        data = await helpers.request_tx_extra_data(tx_req, ofs, size, prev_hash)
+            size = min(1024, tx.extra_data_len - ofs)
-        writers.write_bytes(txh, data)
+            data = await helpers.request_tx_extra_data(tx_req, ofs, size, prev_hash)
-        ofs += len(data)
+            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

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;
 

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;
+  expiry = (coin->decred || coin->overwintered) ? msg->expiry : 0;
-  timestamp = msg->timestamp;
+  timestamp = coin->timestamp ? msg->timestamp : 0;
-  branch_id = msg->branch_id;
+  if (coin->overwintered) {
-  // set default values for Zcash if branch_id is unset
+    version_group_id = msg->version_group_id;
-  if (coin->overwintered && (branch_id == 0)) {
+    branch_id = msg->branch_id;
-    switch (version) {
+    if (branch_id == 0) {
-      case 3:
+      // set default values for Zcash if branch_id is unset
-        branch_id = 0x5BA81B19;  // Overwinter
+      switch (version) {
-        break;
+        case 3:
-      case 4:
+          branch_id = 0x5BA81B19;  // Overwinter
-        branch_id = 0x76B809BB;  // Sapling
+          break;
-        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;

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

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.")