core/sign_tx: add checks for version_group_id and branch_id fields

core/src/apps/bitcoin/sign_tx/helpers.py

@@ -199,6 +199,13 @@ def sanitize_sign_tx(tx: SignTx, coin: CoinInfo) -> SignTx:
         raise wire.DataError("Timestamp must be set.")
     elif not coin.timestamp and tx.timestamp:
         raise wire.DataError("Timestamp not enabled on this coin.")
+    if coin.overwintered and tx.version_group_id is None:
+        raise wire.DataError("Version group ID must be set.")
+    elif not coin.overwintered:
+        if tx.version_group_id is not None:
+            raise wire.DataError("Version group ID not enabled on this coin.")
+        if tx.branch_id is not None:
+            raise wire.DataError("Branch ID not enabled on this coin.")
     return tx
 
 
@@ -219,6 +226,13 @@ def sanitize_tx_meta(tx: TransactionType, coin: CoinInfo) -> TransactionType:
         raise wire.DataError("Timestamp must be set.")
     elif not coin.timestamp and tx.timestamp:
         raise wire.DataError("Timestamp not enabled on this coin.")
+    if coin.overwintered and tx.version_group_id is None:
+        raise wire.DataError("Version group ID must be set.")
+    elif not coin.overwintered:
+        if tx.version_group_id is not None:
+            raise wire.DataError("Version group ID not enabled on this coin.")
+        if tx.branch_id is not None:
+            raise wire.DataError("Branch ID not enabled on this coin.")
     return tx
 
 

tests/device_tests/test_msg_signtx.py

@@ -1022,7 +1022,13 @@ class TestMsgSigntx:
 
     @pytest.mark.parametrize(
         "field, value",
-        (("extra_data", b"hello world"), ("expiry", 9), ("timestamp", 42)),
+        (
+            ("extra_data", b"hello world"),
+            ("expiry", 9),
+            ("timestamp", 42),
+            ("version_group_id", 69),
+            ("branch_id", 13),
+        ),
     )
     @pytest.mark.skip_ui
     def test_prevtx_forbidden_fields(self, client, field, value):
@@ -1045,7 +1051,10 @@ class TestMsgSigntx:
                 client, "Bitcoin", [inp0], [out1], prev_txes={TXHASH_157041: prev_tx}
             )
 
-    @pytest.mark.parametrize("field, value", (("expiry", 9), ("timestamp", 42)))
+    @pytest.mark.parametrize(
+        "field, value",
+        (("expiry", 9), ("timestamp", 42), ("version_group_id", 69), ("branch_id", 13)),
+    )
     @pytest.mark.skip_ui
     def test_signtx_forbidden_fields(self, client, field, value):
         inp0 = proto.TxInputType(

tests/device_tests/test_msg_signtx_zcash.py

@@ -17,6 +17,7 @@
 import pytest
 
 from trezorlib import btc, messages as proto
+from trezorlib.exceptions import TrezorFailure
 from trezorlib.tools import parse_path
 
 from ..tx_cache import TxCache
@@ -133,3 +134,29 @@ class TestMsgSigntxZcash:
             serialized_tx.hex()
             == "0400008085202f890168039326c180fa7b1e999392e25a3ec6a8aec83c11b787ddb1746922020682e3000000006b483045022100f28298891f48706697a6f898ac18e39ce2c7cebe547b585d51cc22d80b1b21a602201a807b8a18544832d95d1e3ada82c0617bc6d97d3f24d1fb4801ac396647aa880121030e669acac1f280d1ddf441cd2ba5e97417bf2689e4bbec86df4f831bf9f7ffd0ffffffff016c9be111000000001976a9145b157a678a10021243307e4bb58f36375aa80e1088ac00000000000000000000000000000000000000"
         )
+
+        @pytest.mark.skip_ui
+        def test_version_group_id_missing(self, client):
+            inp1 = proto.TxInputType(
+                # tmQoJ3PTXgQLaRRZZYT6xk8XtjRbr2kCqwu
+                address_n=parse_path("m/44h/1h/0h/0/0"),
+                amount=300000000,
+                prev_hash=TXHASH_e38206,
+                prev_index=0,
+            )
+            out1 = proto.TxOutputType(
+                address="tmJ1xYxP8XNTtCoDgvdmQPSrxh5qZJgy65Z",
+                amount=300000000 - 1940,
+                script_type=proto.OutputScriptType.PAYTOADDRESS,
+            )
+
+            details = proto.SignTx(version=4)
+            with pytest.raises(TrezorFailure, match="Version group ID must be set."):
+                btc.sign_tx(
+                    client,
+                    "Zcash Testnet",
+                    [inp1],
+                    [out1],
+                    details=details,
+                    prev_txes=TX_API,
+                )